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

CLOUDP-237245: Add example for retrying requests #294

Merged
merged 19 commits into from Mar 28, 2024
Merged

Conversation

wtrocki
Copy link
Member

@wtrocki wtrocki commented Mar 26, 2024

Description

Adding example that would support
Tested manually by using API mocks (thanks to @lantoli) for different network issues and status codes.
All works end to end.

Testing

go run ./examples/retry/retry.go      
2024/03/26 20:43:41 [DEBUG] GET https://cloud-dev.mongodb.com/api/atlas/v2/groups?includeCount=true&itemsPerPage=1&pageNum=1
2024/03/26 20:43:41 [DEBUG] GET https://cloud-dev.mongodb.com/api/atlas/v2/groups?includeCount=true&itemsPerPage=1&pageNum=1
2024/03/26 20:43:41 [DEBUG] GET https://cloud-dev.mongodb.com/api/atlas/v2/groups?includeCount=true&itemsPerPage=1&pageNum=1
2024/03/26 20:43:42 [DEBUG] GET https://cloud-dev.mongodb.com/api/atlas/v2/groups?includeCount=true&itemsPerPage=1&pageNum=1

@wtrocki wtrocki requested a review from a team as a code owner March 26, 2024 15:59
go.mod Outdated Show resolved Hide resolved
@wtrocki wtrocki marked this pull request as draft March 26, 2024 16:27
@wtrocki wtrocki marked this pull request as ready for review March 26, 2024 19:54
@wtrocki
Copy link
Member Author

wtrocki commented Mar 26, 2024

12f3972 was added to not got pinged on dependabot updates for examples. No strong opinion if we want that and we can easily remove this commit if needed.

examples/go.mod Outdated Show resolved Hide resolved
examples/go.mod Outdated Show resolved Hide resolved
examples/retry/retry.go Outdated Show resolved Hide resolved
examples/retry/retry.go Outdated Show resolved Hide resolved
examples/retry/retry.go Outdated Show resolved Hide resolved
internal/main.go Outdated Show resolved Hide resolved
@gssbzn gssbzn changed the title CLOUDP-237245: Add support for retrying requests CLOUDP-237245: Add example for retrying requests Mar 27, 2024
@wtrocki
Copy link
Member Author

wtrocki commented Mar 27, 2024

@gssbzn Based on your feedback we have:

  • Added multi mod support in to the repository
  • Moved example specific dependencies to example project
  • Extended this PR scope to update golang version of repository
  • Updated and reviewed golang CI lint version and configuration.

Let me know if I miss anything

examples/README.md Outdated Show resolved Hide resolved
examples/go.mod Outdated Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

is testify still needed here, could we go mod tidy?

Copy link
Member Author

Choose a reason for hiding this comment

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

We do:

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"

examples/retry/retry.go Outdated Show resolved Hide resolved
examples/retry/retry.go Outdated Show resolved Hide resolved
examples/retry/retry.go Outdated Show resolved Hide resolved
wtrocki and others added 4 commits March 27, 2024 18:28
Co-authored-by: Gustavo Bazan <gustavo.bazan@mongodb.com>
Co-authored-by: Gustavo Bazan <gustavo.bazan@mongodb.com>
Co-authored-by: Gustavo Bazan <gustavo.bazan@mongodb.com>
@wtrocki
Copy link
Member Author

wtrocki commented Mar 27, 2024

I would prefer if we do OpenAPI generator update in the follow up PR as this might involve +60 different file changes.

@wtrocki wtrocki requested a review from gssbzn March 28, 2024 09:28
Copy link
Collaborator

@gssbzn gssbzn left a comment

Choose a reason for hiding this comment

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

LGTM

@wtrocki wtrocki merged commit 5ded14a into main Mar 28, 2024
2 checks passed
@wtrocki wtrocki deleted the CLOUDP-237245 branch March 28, 2024 10:37
wtrocki added a commit that referenced this pull request Mar 28, 2024

### Retry Example

Example provides automatic retries for all HTTP 500, 429 HTTP status errors.
Copy link
Member

@lantoli lantoli Apr 1, 2024

Choose a reason for hiding this comment

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

@wtrocki @gssbzn note that for instance cluster creation can throw 500 but still created the cluster so next call will be DUPLICATE_CLUSTER_NAME 400 that will be a bit confusing for clients.
retries are mostly useful when operations are atomic (happen completely or don't happen at all)

Copy link
Collaborator

Choose a reason for hiding this comment

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

yes, personally blindly retrying on the Atlas API is not safe and a bad idea

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.

None yet

3 participants