-
Notifications
You must be signed in to change notification settings - Fork 3
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
feat: include localhost and canary env as allowed origins for staging backend #71
Conversation
dfc812c
to
31a3bb0
Compare
Codecov Report
@@ Coverage Diff @@
## main #71 +/- ##
==========================================
- Coverage 71.75% 71.72% -0.03%
==========================================
Files 126 126
Lines 10026 10030 +4
==========================================
Hits 7194 7194
- Misses 2832 2836 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
allowed_origins = [f"{web_base_url_parse.scheme}://{web_base_url_parse.netloc}"] | ||
if os.getenv('DEPLOYMENT_STAGE') in ["Staging"]: | ||
allowed_origins.extend([ | ||
"https://canary-cellxgene.dev.single-cell.czi.technology/", |
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 auto-deployed canary env is https://cellxgene.canary.single-cell.czi.technology. Should we use that instead? Thank 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.
Sorry where are you seeing that? https://canary-cellxgene.dev.single-cell.czi.technology/
is set as the web base url in the config so I think thats what we should be using?
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 is the auto deploy GH flow I was referring to!
https://github.com/chanzuckerberg/single-cell-infra/runs/3547390406?check_suite_focus=true
I also see canary
and canary-hook
, so not sure if we only need one or both 😆
https://github.com/chanzuckerberg/single-cell-infra/actions?page=3
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.
So canary hook is sent from the explorer repo to the single-cell-infra repo when code is merged into the main-canary branch. Single-cell-infra then kicks off the deployment in the canary step
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.
Ahh got it, that's why both are needed! Thanks, Madison 🙆♂️
So you're saying https://canary-cellxgene.dev.single-cell.czi.technology/ is the right URL, and the smoke test is just pointing to the wrong one (https://cellxgene.canary.single-cell.czi.technology?
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
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.
LGTM! I think the discussion for Canary env is not a blocker, since we still need to hear back from Emanuele + Infra on rdev anyway 😆 Thank you!! 🚀 🚀
allowed_origins = [f"{web_base_url_parse.scheme}://{web_base_url_parse.netloc}"] | ||
if os.getenv('DEPLOYMENT_STAGE') in ["Staging"]: | ||
allowed_origins.extend([ | ||
"https://canary-cellxgene.dev.single-cell.czi.technology/", |
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.
Ahh got it, that's why both are needed! Thanks, Madison 🙆♂️
So you're saying https://canary-cellxgene.dev.single-cell.czi.technology/ is the right URL, and the smoke test is just pointing to the wrong one (https://cellxgene.canary.single-cell.czi.technology?
Reviewers
Functional:
Readability:
Changes