chore: Update default endpoint URL #30
Conversation
amarsjac
changed the title
| @@ -26,8 +26,7 @@ | |||
| # Environment variable that, if set, overrides the value of CONFIG_FILE_PATH | |||
| CONFIG_FILE_PATH_ENV_VAR = "DEADLINE_CONFIG_FILE_PATH" | |||
| # The default Amazon Deadline Cloud endpoint URL | |||
| # TODO: This is currently set to our closed-beta endpoint. We need to remove this for GA. | |||
| DEFAULT_DEADLINE_ENDPOINT_URL = "https://btpdb6qczg.execute-api.us-west-2.amazonaws.com" | |||
| DEFAULT_DEADLINE_ENDPOINT_URL = "https://deadline.us-west-2.amazonaws.com" | |||
There was a problem hiding this comment.
We shouldn't hardcode the region to us-west-2 here. Also we should just be able to drop the endpoint URL altogether as the AmazonBeaLineModel will generate an SDK that autogenerates the URL altogether. All we need to provide to the client is the region
There was a problem hiding this comment.
+1 is there a reason that we shouldn't make the default None rather than a specific URL? We should be obtaining the endpoint URL from the model.
There was a problem hiding this comment.
We should remove this field entirely from the CLI, config file, and the UI. I was recently looking into other options, and found that https://docs.aws.amazon.com/sdkref/latest/guide/feature-ss-endpoints.html works: you can set an environment variable AWS_ENDPOINT_URL_DEADLINE=https://... and any deadline service calls will use it. (Note: that env var does not work on awscli v1, only awscli v2 and boto3.)
There was a problem hiding this comment.
I've removed the hardcoded URL the URL endpoint is now fetched from the en var AWS_ENDPOINT_URL_DEADLINE I have also stripped the UI element
amarsjac
changed the title
| @@ -26,8 +26,7 @@ | |||
| # Environment variable that, if set, overrides the value of CONFIG_FILE_PATH | |||
| CONFIG_FILE_PATH_ENV_VAR = "DEADLINE_CONFIG_FILE_PATH" | |||
| # The default Amazon Deadline Cloud endpoint URL | |||
| # TODO: This is currently set to our closed-beta endpoint. We need to remove this for GA. | |||
| DEFAULT_DEADLINE_ENDPOINT_URL = "https://btpdb6qczg.execute-api.us-west-2.amazonaws.com" | |||
| DEFAULT_DEADLINE_ENDPOINT_URL = "https://deadline.us-west-2.amazonaws.com" | |||
There was a problem hiding this comment.
+1 is there a reason that we shouldn't make the default None rather than a specific URL? We should be obtaining the endpoint URL from the model.
| @@ -26,8 +26,7 @@ | |||
| # Environment variable that, if set, overrides the value of CONFIG_FILE_PATH | |||
| CONFIG_FILE_PATH_ENV_VAR = "DEADLINE_CONFIG_FILE_PATH" | |||
| # The default Amazon Deadline Cloud endpoint URL | |||
| # TODO: This is currently set to our closed-beta endpoint. We need to remove this for GA. | |||
| DEFAULT_DEADLINE_ENDPOINT_URL = "https://btpdb6qczg.execute-api.us-west-2.amazonaws.com" | |||
| DEFAULT_DEADLINE_ENDPOINT_URL = "https://deadline.us-west-2.amazonaws.com" | |||
There was a problem hiding this comment.
We should remove this field entirely from the CLI, config file, and the UI. I was recently looking into other options, and found that https://docs.aws.amazon.com/sdkref/latest/guide/feature-ss-endpoints.html works: you can set an environment variable AWS_ENDPOINT_URL_DEADLINE=https://... and any deadline service calls will use it. (Note: that env var does not work on awscli v1, only awscli v2 and boto3.)
amarsjac
force-pushed
the
amarsjac/update_endpoint_url
branch
2 times, most recently
from
92c14ca
to
77333f9
Compare
Remove Endpoint URL from UI Signed-off-by: Marshall Jackson <amarsjac@amazon.com>
amarsjac
force-pushed
the
amarsjac/update_endpoint_url
branch
from
77333f9
to
0d4821d
Compare
amarsjac
dismissed
mwiebe’s stale review
Got approval to dismiss via slack, he is AFK and this is time sensitive
| @@ -114,7 +124,7 @@ def get_boto3_client(service_name: str, config: Optional[ConfigParser] = None) - | |||
| session = get_boto3_session(config=config) | |||
|
|
|||
| if service_name == "deadline": | |||
| deadline_endpoint_url = get_setting("settings.deadline_endpoint_url", config=config) | |||
| deadline_endpoint_url = get_deadline_endpoint_url(config=config) | |||
| client = session.client(service_name, endpoint_url=deadline_endpoint_url) | |||
There was a problem hiding this comment.
Should we just omit endpoint_url entirely from this (and other places which use the enpoint url) if it's not in an env var? Then it would default to the service model installed right?
There was a problem hiding this comment.
From marks comment it appears that we don't even need the logic to set in our calls it if AWS_ENDPOINT_URL_DEADLINE is set as an env var because awscliv2 and boto3 handle it
There was a problem hiding this comment.
I see this PR has been merged. Can we address this in a follow up? @amarsjac
What was the problem/requirement? (What/Why)
default endpoint url needs to be updated
What was the solution? (How)
Update with new url endpoint
What is the impact of this change?
service targetgets
How was this change tested?
via unit tests
Was this change documented?
no
Is this a breaking change?
no