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

Add retries for get cluster credentials API call and remove unused variables #107

Merged
merged 2 commits into from
Sep 22, 2023

Conversation

sathiish-kumar
Copy link
Contributor

…riables

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@karle-nishi
Copy link
Contributor

The branch has merge conflicts

@jiezhen-chen
Copy link
Contributor

I see that retries is always set to 10. Do we want to make the number of retries configurable to the end user? To me retries are super valuable when there is operational or connection error that can be solved by simply retrying, but sometimes we also want it to fail fast and fail early, such as when the user configuration is wrong.

@sathiish-kumar
Copy link
Contributor Author

@karle-nishi resolved the merge conflicts.

@jiezhen-chen - I was thinking that we could keep it at 10 for now and then pivot if there is a need for customization. i.e. I'm not completely sure this needs to even be exposed to the outside world.

@sathiish-kumar sathiish-kumar merged commit 120eb0f into main Sep 22, 2023
2 checks passed
@sathiish-kumar sathiish-kumar deleted the add_retries_for_get_cluster_creds branch September 22, 2023 17:11
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.

3 participants