-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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: Adding retry for clone errors #3933
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis update modifies the download source routines to use a context parameter for improved cancellation and deadline management. It renames several parameters for clarity and wraps error handling in a context-aware method. Additionally, the documentation has been updated with revised pre-release naming conventions and a new section on handling errors during source fetching, complete with retry mechanism examples. Test files have also been updated with an added JSON key and a new test for invalid remote downloads using retry logic. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client (with Context)
participant DTF as DownloadTerraformSourceIfNecessary
participant DS as downloadSource
participant EH as RunWithErrorHandling
C->>DTF: Call function with ctx, src, opts, cfg
DTF->>DS: Invoke downloadSource(ctx, src, opts, cfg)
DS->>EH: Execute getter.GetAny with context awareness
EH-->>DS: Return result/error
DS-->>DTF: Propagate download status
DTF-->>C: Return final result
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/integration_tofu_state_encryption_test.go (1)
78-192
: Consider adding test cases forhcl_fmt
behavior.While the JSON structure looks good, we might want to add test cases that verify the behavior when
hcl_fmt
has different values (not justnil
). This would help catch any formatting-related issues early.Here's a suggestion for an additional test case:
// Test with hcl_fmt enabled if generateBlock, ok := generateBlocks.(map[string]interface{})["provider"].(map[string]interface{}); assert.True(t, ok) { assert.Equal(t, true, generateBlock["hcl_fmt"]) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/integration_tofu_state_encryption_test.go
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build-and-test
- GitHub Check: unessential
- GitHub Check: Pull Request has non-contributor approval
🔇 Additional comments (2)
test/integration_tofu_state_encryption_test.go (2)
167-167
: Looking good! Nice consistency update.The addition of
"hcl_fmt": nil
aligns this test with similar changes in other test files. It's a small but important change for maintaining consistent JSON structure across the test suite.
194-220
: Nice helper function! 👍The
validateStateIsEncrypted
helper is well-written and thorough. It checks both the presence and format of encrypted data, making the tests more robust.
Description
Adds support for using the
errors
block to control how errors during source cloning are handled.TODOs
Read the Gruntwork contribution guidelines.
Release Notes (draft)
Updated
errors
block to support controlling how errors during source cloning are handled.Summary by CodeRabbit
Documentation
Tests