[DO NOT MERGE] Rewrite learning uploader #1421
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Is actually path to folder containing notebook
Except for tests...
frankharkins
requested review from
Eric-Arellano and
arnaucasau
and removed request for
Eric-Arellano
frankharkins
had a problem deploying
to
Learning platform (staging)
GitHub Actions
Failure
…H/new-learning-uploader
frankharkins
had a problem deploying
to
Learning platform (staging)
GitHub Actions
Failure
frankharkins
had a problem deploying
to
Learning platform (staging)
GitHub Actions
Failure
arnaucasau
reviewed
May 29, 2024
Co-authored-by: Arnau Casau <47946624+arnaucasau@users.noreply.github.com>
frankharkins
changed the title
|
This is ready for final review, but it might be best to avoid merging until I get back in case there are teething issues. |
Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com>
Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com>
frankharkins
changed the title
frankharkins
changed the title
|
This is ready for final review, but we can't merge until the token has the right permissions. |
arnaucasau
approved these changes
Jun 11, 2024
Mostly paranoia, but changing a slug will probably not have the intended effect. I imagine a contributor would think this would change the slug of the page, but it will actually just upload a new page with the new slug so there would be two pages online. Changing a slug also requires a redirect, for which there's no way to automate. I think a comment here is appropriate as contributors might not (and shouldn't have to) read the entire contributing document before editing this file.
|
This comment is a reminder to update |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The requirements for the learning uploader have changed since it was first created.
It was originally intended to be used locally by writers to quickly push content to the platform. I wrote it in Python so writers would not need to install node, and using string manipulations to interact with the REST API was good enough for just uploading the zip and linking the new file. It also included functionality for a writer to log into the database through a CLI.
Now, the uploader is only used in CI, so we don't care about logging in or sticking to Python, and we want to be able to do more complex manipulations such as adding new lessons and changing metadata. For this reason, this PR rewrites the uploader using the official Directus SDK, which makes interacting with the API a lot easier.