-
Notifications
You must be signed in to change notification settings - Fork 6
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
Feature/maappy gets username #108
Merged
Merged
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
This was referenced Oct 26, 2024
This was referenced Oct 28, 2024
marjo-luc
approved these changes
Oct 29, 2024
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.
Github Issues: MAAP-Project/Community#1055 MAAP-Project/Community#1052
Description
maap-py no longer needs to worry about the username for listing jobs (because the API gets the user from the PGT token). Submitting jobs shouldn't accept user input for the username because they could put someone else's username. Instead, submitting a job fetches the username from the users PGT token. If they don't have a valid PGT token, the job won't submit anyway
Previously, you could submit a job under an invalid username and then the submit wouldn't show up under your account, so we want to remove this possibility
https://api.dit.maap-project.org/api/dps/job/list just needs the PGT token and not username anymore. I removed username as an argument because you should not be able to view other users' jobs (right?) and you don’t need to pass your own username to view your own jobs
Note that once these PRs are merged, this issue: MAAP-Project/Community#1132 addresses that the API should get the username and not maap-py
Overview of work done
Removed username for listing jobs and submitting jobs gets username from PGT token now
Overview of verification done
Tested:
MAAP_API_HOST
, orMAAP_ADE_HOST
in the ADE and with local developmentSome expected behavior examples: If you have the DIT PGT token and
MAAP_API_HOST=api.dit.maap-project.org
and do a badMAAP_ADE_HOST
, you can see your DIT jobs still. If you have a DIT PGT token andMAAP_API_HOST=api.maap-project.org
, you cannot see or submit your jobs. If you submit a job with PGT andMAAP_API_HOST
matching environments, but you add a username field that isn’t yours, it will be overriden to submit a job with your username (soon we will edit the API so it doesn’t even require a username)Overview of integration done
Tests listed above and can test on the ADEs with this image: 'mas.dit.maap-project.org/root/maap-workspaces/jupyterlab/python:remove-env-json'
PR checklist:
poetry run pylint maap
)Draft PR to update documentation: MAAP-Project/maap-documentation#453
See Pull Request Review Checklist for pointers on reviewing this pull request
Goes with these PRs: MAAP-Project/jupyter-server-extension#14 MAAP-Project/dps-jupyter-extension#23