Skip to content
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

Don't call get_connection from provide_bucket_name #28716

Merged

Conversation

dstandish
Copy link
Contributor

@dstandish dstandish commented Jan 4, 2023

We can use the cached connection instead of retrieving the conn object every time.

@Taragolis any reason not to add schema to the wrapper?

We can used the cached connection instead of retrieving the conn object every time.
@Taragolis
Copy link
Contributor

Taragolis commented Jan 4, 2023

@Taragolis any reason not to add schema to the wrapper?

No problem to add this parameter to wrapper. Initially every Hook which use aws_conn did not use it directly or this undocumented feature.

I think better (long term solution) is create some aws-service specific parameter - dictionary where key is aws service and use it for this config, e.g. S3 Bucket or EMR Cluster config or endpoint_url for specific host.

But for now it is fine to just add it to schema name.

@dstandish
Copy link
Contributor Author

dstandish commented Jan 4, 2023

@Taragolis any reason not to add schema to the wrapper?

No problem to add this parameter to wrapper. Initially every Hook which use aws_conn did not use it directly or this undocumented feature.

I think better (long term solution) is create some aws-service specific parameter - dictionary where key is aws service and use it for this config, e.g. S3 Bucket or EMR Cluster config or endpoint_url for specific host.

But for now it is fine to just add it to schema name.

Cool. Thanks. if you don't have any concerns perhaps you can give this one an approval

Copy link
Contributor

@Taragolis Taragolis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@dstandish
Copy link
Contributor Author

thanks @Taragolis

@dstandish dstandish merged commit 9ab9c18 into apache:main Jan 4, 2023
@dstandish dstandish deleted the provide-bucket-dec-no-get-connection branch January 4, 2023 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:providers provider:amazon-aws AWS/Amazon - related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants