-
Notifications
You must be signed in to change notification settings - Fork 716
add support for boto3 kwargs to timestream.create_table #1819
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
Conversation
awswrangler/timestream.py
Outdated
_logger: logging.Logger = logging.getLogger(__name__) | ||
|
||
_BOTOCORE_LOADER = Loader() | ||
_TIMESTREAM_JSON_MODEL = _BOTOCORE_LOADER.load_service_model(service_name="timestream-write", type_name="service-2") |
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.
Just curious is it possible to reuse the service model loaded by boto itself and avoid loading it again? If not, I would prefer not to do this as it adds an overhead purely for validating the arguments which will be anyhow done by boto at later stage
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.
Unsure I can get to what boto is loading for itself, however I hear your point that boto will handle arguments validation anyway. I'm ok removing it entirely. This brings another question though, should we remove it from s3/_fs.py too eventually?
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.
Good point, I wasn't aware we were doing that in the first place... IMO we should remove it as well but I understand this will be a breaking change for the users that pass some unused args that are filtered out and ignored so maybe in 3.0... happy to hear what others think
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Unit tests look OK - the failure is related to Athena |
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.
Looks great!
Feature or Bugfix
Detail
Relates
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.