Fix HttpError 412 error when provisioning access to GCP resources for users #1822
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
tompollard
approved these changes
Jan 23, 2023
physionet-django/notification/templates/notification/email/notify_gcp_access_request.html
Show resolved
Hide resolved
… user The grant_aws_open_data_access and grant_gcp_group_access utility functions return a message if they succeed or fail. The project view was checking this message against custom strings to determine whether to provide a success or error modal to inform the user. This commit changes these functions and the view to use a boolean variable which seems less fragile.
…ess fails Currently, if the system is unable to grant access to a user e-mail to GCP, i.e. unable to add their cloud e-mail to a Google Group, the server will raise an error. Instead, it would be better to notify the user, and provide some advice on what the cause of the error may be.
… google Google Admin sometimes returns a 412. It's not 100% clear, but this seems to happen if the server tries to add a non-Google e-mail to the group. Typically, this occurs if the user has added a non-Google e-mail in their cloud profile. This commit adds a notification to hint the user towards a solution.
alistairewj
force-pushed
the
fix_gcp_group_error
branch
from
7dc27f8
to
e54860b
Compare
|
I refactored the utility functions for clarity and to fix the bug you spotted. The last commit has the changes done in the refactor if you are interested. |
It's a bit brittle to require returning correctly within each if block. Instead, code flow is expected to reach the end of the function, where the message and boolean access variable are returned.
alistairewj
force-pushed
the
fix_gcp_group_error
branch
from
e54860b
to
4a84898
Compare
|
Thanks Alistair, looks good! Hopefully @Chrystinne can do some more cleaning up of this later as part of the AWS integration. |
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.
Recently, we had a few error messages along the lines of:
These seem to occur when a user tries to request access to a project on GCP when their cloud e-mail is not a valid Google e-mail. This PR fixes this by: