Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix static deployment #356

Merged
merged 17 commits into from
Aug 7, 2020
Merged

Fix static deployment #356

merged 17 commits into from
Aug 7, 2020

Conversation

agrn
Copy link
Contributor

@agrn agrn commented Jun 25, 2020

@erikmd reported in #288 that it was no longer possible to deploy learn-ocaml statically. This series fix this by checking the version of the server in the front-end. If the application is deployed statically, there is no /version endpoint, so the front-end will know if it needs to ask the user for a token or not.

The API is also modified so some endpoints can take an optional token, but the server itself will forbid any requests without a token. There are also some modifications in the repository generation process.

This does not yet address learn-ocaml-client, though -- it will fail when trying to talk to a static instance of learn-ocaml.

agrn added 11 commits June 24, 2020 10:42
Signed-off-by: Alban Gruin <alban.gruin@univ-tlse3.fr>
This changes the type of the token of `Exercise_index' from `token' to
`token option' so it can be used without a dynamic server.

Signed-off-by: Alban Gruin <alban.gruin@univ-tlse3.fr>
This changes the type of the token of `Exercise' from `token' to
`token option' so it can be used without a dynamic server.

Signed-off-by: Alban Gruin <alban.gruin@univ-tlse3.fr>
Signed-off-by: Alban Gruin <alban.gruin@univ-tlse3.fr>
If learn-ocaml is deployed statically, the URL of the editor should be
exercise.html, and the URL of the playground must be playground.html.
But when deployed with the server, it can use the clean URL.  To
detect it, we check the token: if it does not exists, it means the
application is deployed without server.

Signed-off-by: Alban Gruin <alban.gruin@univ-tlse3.fr>
When learn-ocaml is statically deployed, the buttons related to the
token cannot really be used.  Disable them when they are created, then
enable them once the token is validated.

As `sync_button_state' is unused and not fit for this purpose, it is
replaced by a button group, `sync_button_group'.  The buttons are
associated to this group when they are created.

Signed-off-by: Alban Gruin <alban.gruin@univ-tlse3.fr>
Signed-off-by: Alban Gruin <alban.gruin@univ-tlse3.fr>
The client needs the metadata of an exercise.  The server adds it
dynamically, but in a static scenario, it must be present in the file
in the first place.

Signed-off-by: Alban Gruin <alban.gruin@univ-tlse3.fr>
Signed-off-by: Alban Gruin <alban.gruin@univ-tlse3.fr>
Signed-off-by: Alban Gruin <alban.gruin@univ-tlse3.fr>
Signed-off-by: Alban Gruin <alban.gruin@univ-tlse3.fr>
@agrn
Copy link
Contributor Author

agrn commented Jun 25, 2020

The fail is the macos CI that failed to download some opam packages…

This enables learn-ocaml-client to interact with a static instance of
learn-ocaml by allowing to store no token in the configuration file.

The unused function `init' is also removed.

Signed-off-by: Alban Gruin <alban.gruin@univ-tlse3.fr>
@agrn
Copy link
Contributor Author

agrn commented Jun 25, 2020

I added another patch to fix learn-ocaml-client.

@agrn agrn force-pushed the ag/static-deployment branch from cb12bd3 to 277548a Compare June 29, 2020 12:19
agrn added 2 commits June 29, 2020 14:54
Signed-off-by: Alban Gruin <alban.gruin@univ-tlse3.fr>
Signed-off-by: Alban Gruin <alban.gruin@univ-tlse3.fr>
@agrn agrn force-pushed the ag/static-deployment branch from 3213b11 to 22f7f8f Compare June 29, 2020 12:54
@erikmd
Copy link
Member

erikmd commented Jun 29, 2020

Hi @yurug, I've quickly proofread @agrn's PR #356 and it looks very fine.

However, I'd suggest you'd merge PR #353 beforehand because it is also ready and it will extend the CI.

FYI, I've just deployed the result of this PR in a GitHub user Page here:
https://github.com/pfitaxel/pfitaxel-demo-dev/tree/master/docs
and the result is available here:
https://pfitaxel.github.io/pfitaxel-demo-dev

Note that this feature of static deployment was broken at least from e0a872e, notably because of the leading "/" in the HTML/CSS/JS URLs, which was de-facto incompatible with having a BASE_URL such as https://pfitaxel.github.io/pfitaxel-demo-dev.

With this PR, one can deploy a fully working, statically-served instance of learn-ocaml (except the server-side storage feature) incorporating some exercises from a private GitHub repository associated with a public GitHub Pages deployment.

It basically suffices to run:

$ learn-ocaml build --root "https://pfitaxel.github.io/pfitaxel-demo-dev" --repo=demo-repository

(cf. this example deployment script (that should be rewritten to rely on Docker though))

Signed-off-by: Alban Gruin <alban.gruin@univ-tlse3.fr>
@agrn agrn force-pushed the ag/static-deployment branch from 367cc57 to 15bad51 Compare June 29, 2020 13:28
Signed-off-by: Alban Gruin <alban.gruin@univ-tlse3.fr>
@agrn
Copy link
Contributor Author

agrn commented Jun 29, 2020

Note that it adds a dependency to markup-lwt (we already have markup, though, so it’s not a big step to take).

@erikmd
Copy link
Member

erikmd commented Jun 29, 2020

@yurug CI is green now, although it seems the automatic PR status feedback from Travis → GitHub doesn't seem to work very well :-/

@yurug
Copy link
Collaborator

yurug commented Aug 7, 2020

Thank you!

@yurug yurug merged commit 1332c6e into ocaml-sf:master Aug 7, 2020
@erikmd erikmd mentioned this pull request Aug 7, 2020
@erikmd erikmd deleted the ag/static-deployment branch August 12, 2020 22:19
@gasche
Copy link
Contributor

gasche commented Sep 20, 2020

If I understand correctly, it is now possible to deploy a learn-ocaml instance as a static website. That sounds like something that could be useful for my own teaching this semester.

Is this static deployment mode documented in docs/? I haven't found any information. Could you update docs/howto-deploy-a-learn-ocaml-instance.md to document static deployment?

In the meantime, I'm going to experiment with @erikmd's documentation in the comment above ( #356 (comment) ).

Note: last year I tried to deploy several learn-ocaml instances on the same server, using different base URLs. I sent a couple preliminary PRs, but never managed to finish the work (a lot of URLs in various places had to be fixed, etc.). If I understand correctly, doing this is possible with static deployment; this could be very helpful for me.

@erikmd
Copy link
Member

erikmd commented Nov 24, 2020

Hi @gasche

Is this static deployment mode documented in docs/? I haven't found any information. Could you update docs/howto-deploy-a-learn-ocaml-instance.md to document static deployment?

In the meantime, I'm going to experiment with @erikmd's documentation in the comment above ( #356 (comment) ).

just FYI the deploy script I had mentioned in my earlier comment is now migrated to docker run, so no opam switch or so is needed anymore to build the www folder for learn-ocaml static deployment, and one just need to (install Docker Engine and) run:

$ ./deploy /path/to/learn-ocaml-repository

to generate the www folder, commit, and − if we validate with Return −, push (e.g., to a private github repo if we use GitHub Pages)

Anyway, it's true we'll need to document further that feature before next learn-ocaml release! - I'll try to open a pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants