-
Notifications
You must be signed in to change notification settings - Fork 1
feat: add module to fetch primary_metadata_region #127
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
base: main
Are you sure you want to change the base?
Conversation
ocofaigh
left a comment
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.
Have you run the script in an account where no primary metadata region has been set? What happens?
The script is currently hard coded to production endpoints, meaning it wont work in stage. The script needs to be able to handle environment variable overrides that someone might make. Same way we did in confirm_lb_active.sh for example.
In an account where the
|
|
/run pipeline |
|
/run pipeline |
|
/run pipeline |
|
/run pipeline |
ocofaigh
left a comment
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.
@iamar7 Can you show in todays deep dive for team feedback please?
|
/run pipeline |
|
/run pipeline |
|
/run pipeline |
|
/run pipeline |
|
/run pipeline |
|
/run pipeline |
| import sys | ||
| import time | ||
|
|
||
| import requests |
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.
We cannot use requests - this is an external pip package that requires installation. I did mention this on the deep dive. We can use the python built in http library: https://docs.python.org/3/library/http.client.html
Check out our internal sdnlb module which recently was updated to use this library too.
ocofaigh
left a comment
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.





Description
Resolves: #102
Release required?
x.x.X)x.X.x)X.x.x)Release notes content
Run the pipeline
If the CI pipeline doesn't run when you create the PR, the PR requires a user with GitHub collaborators access to run the pipeline.
Run the CI pipeline when the PR is ready for review and you expect tests to pass. Add a comment to the PR with the following text:
Checklist for reviewers
For mergers