allow user to specify default value when using store #1020
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
mcalhoun
added
the
no-release
Graphite App
mcalhoun
force-pushed
the
add-store-defaults
branch
from
e1b28dd to
2dc79e1
Compare
📝 WalkthroughWalkthroughThe changes introduce a Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Caller
participant P as processTagStore
participant Store as Store Service
participant Logger as Logger
Caller->>P: Invoke processTagStore("storeParams|defaultValue")
P->>P: Split input into storeParams and defaultValue
P->>Store: Lookup value using storeParams
alt Lookup Success
Store-->>P: Return value
P-->>Caller: Return retrieved value
else Lookup Failure
P->>P: Check for provided defaultValue
alt Default value provided
P-->>Caller: Return defaultValue
else No default value
P->>Logger: Log error
P-->>Caller: Return error
end
end
Tip 🌐 Web search-backed reviews and chat
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🔇 Additional comments (4)
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: 1
🧹 Nitpick comments (2)
internal/exec/yaml_func_store.go (2)
29-43: Pipe-based default value parsing is effective.The parsing logic is straightforward and clearly signals invalid formats. Consider more robust parsing if multiple pipes or extra arguments are intended in the future.
67-80: Consider returning errors instead of exiting.Exiting might be limiting for callers who need to handle errors in different ways. A potential refactor:
- u.LogErrorAndExit(fmt.Errorf("failed to get key: %s", err)) + return fmt.Errorf("failed to get key: %w", err)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
internal/exec/yaml_func_store.go(2 hunks)internal/exec/yaml_func_store_test.go(1 hunks)website/docs/core-concepts/stacks/yaml-functions/store.mdx(2 hunks)
🧰 Additional context used
🪛 golangci-lint (1.62.2)
internal/exec/yaml_func_store_test.go
39-39: Error return value of redisStore.Set is not checked
(errcheck)
40-40: Error return value of redisStore.Set is not checked
(errcheck)
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: [mock-windows] tests/fixtures/scenarios/complete
- GitHub Check: [mock-windows] examples/demo-vendoring
- GitHub Check: [mock-windows] examples/demo-component-versions
- GitHub Check: Acceptance Tests (macos-latest, macos)
- GitHub Check: Acceptance Tests (windows-latest, windows)
- GitHub Check: Acceptance Tests (ubuntu-latest, linux)
- GitHub Check: [localstack] demo-localstack
- GitHub Check: [k3s] demo-helmfile
- GitHub Check: Summary
🔇 Additional comments (7)
internal/exec/yaml_func_store.go (2)
14-18: Pointer-based default value field looks good.Using a pointer to differentiate between having no default and having an empty default is clean and clear.
45-65: Parameter handling for 3 or 4 arguments is consistent.The logic for assigning stack, component, and key works nicely.
internal/exec/yaml_func_store_test.go (1)
42-98: Tests cover essential scenarios.You have thorough coverage for default handling, invalid formats, cross-stack lookups, and more. Helpful for preventing regressions.
website/docs/core-concepts/stacks/yaml-functions/store.mdx (4)
12-12: Updated remote store reference is clear.Explicitly noting Redis support is helpful.
18-18: Highlighting default value usage.Mentioning two or three parameters plus a default is concise and user-friendly.
26-28: Example with default value is straightforward.Illustrates the syntax well.
42-45: Documentation for 'default_value' parameter aligns with code.This clarifies how to handle missing keys elegantly.
mcalhoun
force-pushed
the
add-store-defaults
branch
from
2f869b1 to
dceeb04
Compare
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
mcalhoun
force-pushed
the
add-store-defaults
branch
from
dceeb04 to
b0b828b
Compare
|
These changes were released in v1.159.0. |
* allow user to specify default value when using store * Update internal/exec/yaml_func_store_test.go Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * allow user to specify default value when using store --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
what
!storeYAML function using the pipe (|) syntaxwhy
references
/core-concepts/projects/configuration/storesSummary by CodeRabbit