-
Notifications
You must be signed in to change notification settings - Fork 607
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
Delete API Gateway if cluster up fails #1172
Conversation
|
||
|
||
def get_api_gateway_endpoint(): | ||
cluster_name = os.environ["CORTEX_CLUSTER_NAME"] |
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.
Since there are only 2 variables, it may be easier to follow the code that calls this python file if these values are accepted as args
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 other python files, e.g. create_gateway_integration.py
, also pull from the environment variables, so I think for now I lean towards leaving it as is. I'm actually leaning toward moving more of the api gateway code from install.sh
to python, so at that point we can convert to only using args, does that sound ok?
|
||
|
||
def get_api_gateway_id(): | ||
cluster_name = os.environ["CORTEX_CLUSTER_NAME"] |
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.
Since there are only 2 variables, it may be easier to follow the code that calls this python file if these values are accepted as args
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.
(see comment above)
manager/get_api_gateway_endpoint.py
Outdated
return api_gateway["ApiEndpoint"] | ||
|
||
raise Exception( | ||
f"Could not find api gateway with tag cortex.dev/cluster-name={cluster_name} in {region}" |
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.
Could not find api gateway with tag
-> Could not get endpoint for api gateway with tag
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 think the current error message is a little more descriptive/accurate, since if this exception is raised, it means that there was not API gateway that had the tag. I'll reword it to make it more clear: "your cluster's api gateway (in {region} with tag cortex.dev/cluster-name={cluster_name}) does not exist". Does that sound ok?
(cherry picked from commit b17c225)
checklist:
make test
andmake lint