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

feat(athena-adapter): support custom retry configurations for boto3 calls #494

Merged
merged 2 commits into from
Nov 9, 2023

Conversation

lukealexmiller
Copy link
Contributor

Description

This PR introduces a new functionality to the Athena adapter allowing users to specify a custom number of retry attempts for boto3 calls within the dbt-athena-community plugin. The modification adds a parameter to get_boto3_config function, enabling it to accept the number of retries as an argument. This change enhances resilience and configurability by letting users define their retry strategies, accommodating for network inconsistencies or AWS rate limits.
Resolves #480.

Changes:

  • Added a num_retries parameter to get_boto3_config function to accept custom retry attempts.
  • Introduced num_boto3_retries as an optional configuration in AthenaCredentials to store the number of boto3 retries.
  • get_effective_num_retries method in AthenaCredentials to determine the effective number of retries considering the optional num_boto3_retries.
  • Altered calls to get_boto3_config within AthenaConnectionManager and AthenaAdapter to pass the effective number of retries.
  • Updated unit test test_get_boto3_config to validate the retry configuration within the boto3 config.

The default behavior remains unchanged if the num_boto3_retries is not set, with the system defaulting to the existing num_retries value.

Testing:

The updated test ensures that the user_agent_extra remains correctly set and validates the new retry configuration is applied when num_boto3_retries is specified.

Note:

Ensure your environment configuration aligns with the new changes by providing the num_boto3_retries value if custom retry behavior is desired.

Models used to test - Optional

Checklist

  • You followed contributing section
  • You kept your Pull Request small and focused on a single feature or bug fix.
  • You added unit testing when necessary
  • You added functional testing when necessary

@nicor88
Copy link
Contributor

nicor88 commented Nov 8, 2023

Overall looks good.
@svdimchenko and @Jrmyy could you have another review?
@lukealexmiller please rebase your branch with the latest main in dbt-athena, thanks.

Copy link
Contributor

@svdimchenko svdimchenko left a comment

Choose a reason for hiding this comment

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

@lukealexmiller everything looks nice to me, just let's fix minor comments about naming and docs

@nicor88
Copy link
Contributor

nicor88 commented Nov 9, 2023

@lukealexmiller do you think that you can make the changes that @svdimchenko requested within the day? then we include this in the next release.

Copy link
Contributor

@Jrmyy Jrmyy left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution
Except @svdimchenko comments LGTM ✅

@lukealexmiller lukealexmiller force-pushed the support-custom-retry-configuration branch from 80c2467 to ac351a5 Compare November 9, 2023 15:02
@lukealexmiller
Copy link
Contributor Author

Thanks for the feedback all, hopefully this is now ready in time for the next release! btw, most of the doc changes are due to markdownlint in the pre-commit check.

@nicor88 nicor88 merged commit 48967f7 into dbt-labs:main Nov 9, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Materialized Tables - Error: (SlowDown) when calling the DeleteObjects operation
4 participants