update aws param store store to use getKey #1009
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This stack of pull requests is managed by Graphite. Learn more about stacking. |
This was referenced Feb 3, 2025
Merged
mcalhoun
force-pushed
the
aws-ssm-param-store-get-key
branch
from
2ad5ece to
5a28ac1
Compare
mcalhoun
force-pushed
the
add-redis-store
branch
from
e0eb984 to
fd28158
Compare
mcalhoun
force-pushed
the
aws-ssm-param-store-get-key
branch
from
7411b68 to
8563f33
Compare
mcalhoun
force-pushed
the
add-redis-store
branch
2 times, most recently
from
7494ec7 to
ecae929
Compare
mcalhoun
force-pushed
the
aws-ssm-param-store-get-key
branch
from
8563f33 to
13ffe7f
Compare
mcalhoun
force-pushed
the
aws-ssm-param-store-get-key
branch
from
f94f4cd to
b4816f7
Compare
📝 WalkthroughWalkthroughThe changes enhance the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Set/Get Method
participant SSM as SSMStore.getKey
participant Helper as Key Constructor
Caller->>SSM: Request key generation (stack, component, key)
SSM->>SSM: Check if stackDelimiter is non-nil
alt stackDelimiter is nil
SSM-->>Caller: Return error
else
SSM->>Helper: Build key using prefix and delimiter
Helper-->>SSM: Return constructed key
SSM-->>Caller: Return key, nil error
end
Suggested labels
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
pkg/store/aws_ssm_param_store.go (1)
37-37: Consider enhancing context handling in future updates.Currently using
context.TODO()as a placeholder. Consider:
- Accepting context as a parameter in these methods
- Adding timeout control for AWS operations
Also applies to: 90-90, 130-130
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pkg/store/aws_ssm_param_store.go(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Summary
🔇 Additional comments (3)
pkg/store/aws_ssm_param_store.go (3)
99-102: LGTM! Error handling follows best practices.Good use of error wrapping with proper context. 🛡️
133-136: LGTM! Consistent error handling pattern.Maintains symmetry with the Set method's error handling. 🔄
70-76: LGTM! Robust error handling added.The nil check for
stackDelimiterand error propagation are solid improvements. 💪However, I notice that the
getKeyhelper function called on line 75 is not visible in this file.Let's verify its implementation:
✅ Verification successful
Helper function implementation located and verified
- The
getKeyhelper function is implemented inpkg/store/store.gowith the appropriate signature.- The change in error handling is solid, and its dependency on the helper function is correctly managed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the getKey helper function implementation ast-grep --pattern 'func getKey($prefix string, $delimiter string, $stack string, $component string, $key string, $separator string) (string, error)'Length of output: 151
Script:
#!/bin/bash # Use rg to search for any occurrence of 'func getKey(' in Go files across the repository rg "func\\s+getKey\\(" --type goLength of output: 186
osterman
approved these changes
Feb 4, 2025
mcalhoun
force-pushed
the
aws-ssm-param-store-get-key
branch
from
f546259 to
4301172
Compare
This was referenced Feb 4, 2025
|
These changes were released in v1.159.0. |
Cerebrovinny
pushed a commit
that referenced
this pull request
Feb 9, 2025
## what - Refactored the `getKey` method in AWS SSM Parameter Store to handle nil stack delimiters - Added error handling for cases where stack delimiter is not properly set - Improved error propagation in `Set` and `Get` operations ## why - Prevents potential nil pointer dereferences when stack delimiter is not initialized - Makes the code more robust by explicitly handling error cases - Improves debugging capabilities by providing more specific error messages ## references - Related to AWS SSM Parameter Store documentation: https://docs.aws.amazon.com/systems-manager/latest/userguide/systems-manager-parameter-store.html <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved error reporting to promptly capture and inform users of configuration issues. - **Refactor** - Streamlined the process for constructing actual configuration keys, enhancing overall consistency and system reliability. - Enhanced error handling in key retrieval processes for improved robustness. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
what
getKeymethod in AWS SSM Parameter Store to handle nil stack delimitersSetandGetoperationswhy
references
Summary by CodeRabbit
Bug Fixes
Refactor