Skip to content

Conversation

@taniwallach
Copy link
Member

I just made use of bin/OPL_releases/release.sh to create updated OPL metadata files, which are at https://github.com/taniwallach/webwork-open-problem-library/releases/tag/2021-08-03

GitHub reported by email that the script is using a deprecated approach to make use of the access token:

Please use the Authorization HTTP header instead, as using the `access_token` query parameter is deprecated. If this token is being used by an app you don't have control over, be aware that it may stop working as a result of this deprecation.

This PR change the manner in which the token in sent to GitHub, as using to use the Authorization HTTP header instead of the access_token query parameter which was deprecated. See: https://developer.github.com/changes/2019-11-05-deprecated-passwords-and-authorizations-api/

The changes were made in the same manner as in gap-system/ReleaseTools@f8ce1aa#diff-a4d451ec23463726f72c43d64c710968f6b602cd653b4de8adee1b556240a829 on which @heiderich based the code.

I also moved the test of the ARCHIVE_FORMATS environment variable, so when it is necessary to abort, it is done very early on the process (in particular before running OPL-update). Otherwise, someone who forgets to set it would need to either rerun the script, which would invoke the OPL-update again, which is very time-consuming, or "hack" around that.


Process to make a release:

  1. Make sure you have a fork of the OPL, and a GitHub access token with suitable permissions.
  2. If running WeBWorK in Docker: enter bash inside the running Docker container: docker container exec -it webwork2_app_1 bash
  3. Check if python works, and if necessary fix it (or install python)
    • python --version
    • On a current Docker of WeBWorK 2.16 on an Ubuntu 20.04 base) I needed to run ln -s /usr/bin/python3 /usr/bin/python
  4. Run the following commands, after setting suitable values in the call of bin/OPL_releases/release.sh on the last line. You need to provide values for
    • my-tag-name
    • my_github_user
    • my-secret-token
cd /opt/webwork/libraries/webwork-open-problem-library

# Update the OPL:
git fetch origin
git checkout master
git pull

export ARCHIVE_FORMATS=".tar.gz .tar.bz2"

# Set the necessary values in the command below

/opt/webwork/webwork2/bin/OPL_releases/release.sh -t "my-tag-name" -r my_github_user/webwork-open-problem-library --token my-secret-token  --libraryname webwork-open-problem-library

access_token as a query parameter is deprecated.
See: https://developer.github.com/changes/2019-11-05-deprecated-passwords-and-authorizations-api/

Instead use the Authorization HTTP header instead. Changes were made in the
same manner as in gap-system/ReleaseTools@f8ce1aa#diff-a4d451ec23463726f72c43d64c710968f6b602cd653b4de8adee1b556240a829
on which @heiderich based the code.

Also move the test of the ARCHIVE_FORMATS environment variable, so when
it is necessary to abort, it is done very early on the process (in particular
before running OPL-update).
@taniwallach
Copy link
Member Author

Note: The bin/OPL_releases/release.sh only runs bin/OPL-update which does update tagging-taxonomy.json (which seems not to have changed since the last metadata release), but does not update the other JSON files which are now only created when bin/updateOPLextras.pl is run, Those additional files do not seem to be in use, and are apparently related to WW3 and unimplemented plans.

See the discussion in #959

@pstaabp - Could you confirm that webwork2 does not make any use of the three additional JSON files generated by bin/updateOPLextras.pl.

@taniwallach
Copy link
Member Author

The thread at #1118 discussed the outdated metadata being used, and I added manual instructions to update inside a running Docker container.

If possible, we should make a release in the main OPL repository, and update the bin/OPL_releases/Makefile so it will get releases from there. That correction should probably be a hot-fix, so new Docker setups will get a current set of metadata instead of metadata from 2019 from the most recent release at https://github.com/heiderich/webwork-open-problem-library/releases .

@taniwallach taniwallach added Do Not Merge Yet PR to allow others to inspect -- not ready for prime time Undergoing revisions Additional changes are needed in this PR labels Sep 2, 2021
@drgrice1
Copy link
Member

drgrice1 commented Apr 6, 2022

@taniwallach: It would be nice to get this into this release. Most important is to stop using @taniwallach's personal fork of the OPL, and get releases into a more appropriate location.

1 similar comment
@drgrice1
Copy link
Member

drgrice1 commented Apr 6, 2022

@taniwallach: It would be nice to get this into this release. Most important is to stop using @taniwallach's personal fork of the OPL, and get releases into a more appropriate location.

@pstaabp
Copy link
Member

pstaabp commented Apr 21, 2022

Is this similar metadata fetching that we just approved in #1657?

@drgrice1
Copy link
Member

Yes, this is related to that. Basically, this script that is updated here is obsolete now with #1657.

@pstaabp
Copy link
Member

pstaabp commented Apr 21, 2022

Should we remove the script then?

I was going to add that there is a short subroutine to calls python to do some JSON parsing, which seems overkill to require another language especially since perl is plenty happy parsing JSON.

@drgrice1
Copy link
Member

Yeah, I almost deleted the script in #1657. I don't see any real reason to keep it around. Docker still uses the Makefile in that directory though. Docker should be updated to using the bin/download-OPL-metadata-release.pl script, but that script may need a bit of modification to work with the way that Docker does things in docker-entrypoint.sh.

@taniwallach
Copy link
Member Author

Since the OPL now has a GitHub action to create the metadata releases, the whole approach of how they were manually created is outdated. I would say that this script and related things should be removed.

As @drgrice1 wrote - there is a need to change the docker-entrypoint.sh file so that it can use bin/download-OPL-metadata-release.pl without a dependence on the bin/OPL_releases/Makefile which should no longer be needed. The one addition probably needed for Docker setups beyond running the new script is to run bin/load-OPL-global-statistics.pl so that gets loaded (as otherwise the library browser runs into trouble, from what I recall). Maybe that should just be done in bin/download-OPL-metadata-release.pl.

@taniwallach taniwallach closed this Jun 9, 2022
@drgrice1
Copy link
Member

drgrice1 commented Jun 9, 2022

@taniwallach: Actually things have changed even further since I made that comment. Probably we will want to switch to just running OPL-update. However, the OPL-update script will probably no longer work directly the docker-entrypoint.sh script, and will probably need to be fixed for that. The problem is that now the OPL-update also uploads local OPL statistics, and that requires user input. See #1693. Most likely the script is going to need an optional command line flag to skip that step.

@taniwallach taniwallach removed Do Not Merge Yet PR to allow others to inspect -- not ready for prime time Undergoing revisions Additional changes are needed in this PR labels Jun 10, 2022
@taniwallach
Copy link
Member Author

@taniwallach: Actually things have changed even further since I made that comment. Probably we will want to switch to just running OPL-update. However, the OPL-update script will probably no longer work directly the docker-entrypoint.sh script, and will probably need to be fixed for that. The problem is that now the OPL-update also uploads local OPL statistics, and that requires user input. See #1693. Most likely the script is going to need an optional command line flag to skip that step.

Thanks. I think #1722 should work. I used an environment variable to bypass the upload, as that was faster for me to do.

@drgrice1
Copy link
Member

Yeah, I actually thought about it later and was thinking that an environment variable would be better than a command line flag. The point here is to have a bypass method that is somewhat obscure and unadvertised, as we want admin's running this from command line to upload statistics. That was @drdrew42's point with the current setup.

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.

3 participants