WW 2.15 - update Docker config to use Ubuntu 18.04#980
Conversation
|
Warning:
Error messages seen:
... ... ... |
|
@taniwallach : running in the db container corrects the message. The integration of mysql 5.7+ surely adds that... it is usually worst when going to mysql8.0. |
|
I have implemented some of the ideas about the OPL databases raised in:
In particular:
@nmoller @xcompass - Your testing and feedback would be appreciated. If the local where the value of @mgage @heiderich @nmoller @jwj61 : We need to consider how to best distribute pre-made versions of these data files in the future:
Given that these are large files which change quickly as modifications are made to the OPL, it does not seem wise to include then (or gzipped versions) inside the OPL itself. Maybe a pointer to the URL of the current version of a TGZ file of the complete set could be included in the OPL, together with a SHA256 sum or something else to help verify the integrity of the downloaded file. |
Maybe we could use releases of GitHub to store those files. And yes, I also think that we could include a link to the latest "release" of the OPL together with a checksum. I guess at least the latter must be hard coded somewhere. Maybe we could do periodic releases of the OPL. I guess the whole process (creating the tgz, making a release on GitHub and updating the reference in the docker related files) could be automatized. |
I think given the size of the files, at the very least it should be in a separate Git repository, possibly defined as a sub-module of the OPL, so that the old files do not accumulate in the main OPL Git repository. Most people could use a "shallow clone" of that special repository (to only get the current version). If it were a separate Git repository, we could possibly reset it as an empty repository, or change repositories every so often. I tend to think that huge non-human generated files like this are probably not best handled in Git (but there is Git LFS). I think it might be better to host them as static files somewhere else. But, I have not given this very much thought yet. |
|
I did not mean to commit the tgz files. I proposed to add them as additional binary files to the releases (see step 7 in https://help.github.com/en/articles/creating-releases). |
This sounds like a very feasible approach. It certainly provides a means of hosting the tgz file on GitHub, and they don't restrict anything except that each file must be under 2GB. We would need to determine how to make it easy to include the links to those files in the main OPL repository, but that can be handled by small text files. It might still be easier to handle it as a different repository, just to keep the OPL "clean" and not clogged up with pseudo-releases used to distribute the tgz files. |
As far as I understand, the git repository itself would not be affected by the releases and would thus not be "clogged" by the binary files. I guess this might have been a motivation to introduce the release files on GitHub. |
|
I think Florian's idea seems great. We have to put a note to modify the docker-entrypoint script for doing the "plummbing" needed to use those distribution files. |
|
The dump and release could maybe be automated as @mgage suggested in #972 (comment):
I am however not sure whether we really want a new release after every commit or once a day if changes were made. This may also depend on how these releases are used in different contexts. |
heiderich
left a comment
There was a problem hiding this comment.
If the local webwork2 tree has the changes in this PR before it is merged into the WeBWorK-2.15 branch, the image built will contain the updated docker-entrypoint.sh but will not contain the new scripts. To overcome this for testing, I manually mounted them by adding 2 more lines to the volumes section of docker-compose.yml:
This makes testing a bit cumbersome. As I see it, these two scripts are independent from the docker changes, even though they might be particularly useful.
@taniwallach How about opening a new PR in which you only add these two scripts. Then we could test it, merge it and afterwards come back to this PR. Ideally include some instructions on how to test the scripts. Maybe you could even add some a unit test for the scripts (I guess roughly dumping followed by dropping the tables and finally restoring them should not change anything).
I would strongly advise to add such tests whenever possible. I know that this can be a lot of additional work, but I think in the long run we will profit from such tests.
heiderich
left a comment
There was a problem hiding this comment.
Overall the changes look good to me.
|
I'm trying to test this and being new to docker, I get the following error a number of times: I had it running, with |
|
As an additional general comment to my last review: My impression was that certain values are used in several places. In general I prefer avoiding hardcoding values that appear at least twice. The idea of some of the comments in this review is the following: Use variables in the |
|
When is |
…rk2_and_pg introduce build time arguments for the branches of webwork2 and pg
and use it instead of the hard-coded string "../ww-docker-data/courses" in docker-compose.yml
and remove comments on old default
define the environment variable COURSES_DIRECTORY_ON_HOST in .env
… for utf8mb4 - dbMB4
Set them all in docker-compose.yml, where some depend on the .env file.
|
Based on @heiderich's changes, and an off-line discussion, all the They are now defined in |
|
I just tested this PR again after all the recent changed using a clean run of but then manually copied the updated script files from #985 into and everything I tested seems fine to me (login, library browser, dump+restore of OPL tables). However, I kept my data volumes (SQL and courses) unchanged. @heiderich @nmoller - Can you also run a test. If someone can let a totally clean machine without any data volumes start up and run an OPL-update, we can:
The only thing I would like to consider adding to this PR is a download of the OPL table dump and JSON data... so that a Docker system can be brought up without any need to run |
|
I tested it and modulo the missing binaries it seems to work, also with my changes from taniwallach#11. I approve this PR being merged once #985 is merged. |
remove option "--depth 1"
$WEBWORK_ROOT
fix typo in Dockerfile
|
I just merged #985 as we seem to have settled on the current level of security for the DB password there (using the As getting that merged in first was the main thing we we waiting for, I'm merging this in also. Further testing can be done in the WeBWorK-2.15 branch, and any additional improvements can be made later. |
Changes were made, and this was rebased to move the OPL scripts into #985.
The
docker-entrypoint.shfiles assumes that the OPL-scripts from #985 are available, but that is the price of splitting the PR into two independent parts.This replaces the Docker config changes from #972 and was influenced by feedback from @xcompass and @nmoller in that thread and elsewhere.
Changes:
docker-compose.yml. The sample files are in under thedocker-config/subdirectory, anddocker-entrypoint.shwas moved into that subdirectory.docker-compose.yml.SSLcan be used to turn SSL on in the running container.ADD_LOCALEScan be used to have the running container build additional locale files on startup.ADD_PACKAGEScan be used to have the running container install additional Ubuntu packages on startup.PAPERSIZEcan be used to set the default system papersize in the running container.