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.
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
Authenticate endpoints and validate user state_code in metrics requests #546
Authenticate endpoints and validate user state_code in metrics requests #546
Changes from 5 commits
16f3133
03dcea1
d739491
643f214
f6027dd
823b079
552814e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This means we'll need to change the staging environment to make sure these keys are set
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea good call, do you know where/how to do this? I'm actually not sure, I forget where the Spotlight backend is hosted. I see something both in Firebase and also GCP.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the backend is in App Engine, there are GAE config files in a shared 1password document that we have to update and then store locally (untracked) in order to deploy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@colincadams do you know how to set the staging environment variables?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@terryttsai @macfarlandian so is it sufficient to update gae-staging.yaml in public-dashboard-untracked-config-files.zip in 1pass? How do those get pulled in during deploy?
@terryttsai you don't have access to 1pass, right? So I can do this for you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so, I don't see that file in 1pass
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that zip file is the "master copy" but then you have to download it and copy those files into their respective locations in your working directory. (The deployed backend does not actually use the
.env
file, which is a little confusing, it uses the environment config in the yaml for that environment. The.env
is just for development.)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. Are spotlight deploys just done manually by whomever wants to then? By running
gcloud app deploy gae-staging.yaml --project [staging_project_id]
? Or is there a more formal process?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nope that's as formal as it gets
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, ok this all makes sense, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should throw different error messages in these different cases:
Might be helpful for debugging?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe I can assert that
AUTH0_APP_METADATA_KEY
is defined and we'll see the error in the logs if it's not? If any of the rest is empty we know the user app_metadata hasn't been configured properly. We could have different logs though I would want to return the same 401 error to the client.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep sounds good, whatever you think is best! Just thought I'd point it out