Add --query (shorthand -q) flag to all atmos describe <subcommand> commands
#920
Conversation
# Conflicts: # cmd/terraform.go # go.mod # go.sum
|
Important Cloud Posse Engineering Team Review RequiredThis pull request modifies files that require Cloud Posse's review. Please be patient, and a core maintainer will review your changes. To expedite this process, reach out to us on Slack in the |
📝 WalkthroughWalkthroughThis pull request introduces a comprehensive set of changes to the Atmos CLI, focusing on enhancing command functionality, error handling, and configuration management. The primary additions include a new Changes
Sequence DiagramsequenceDiagram
participant User
participant CLI
participant YQProcessor
participant ConfigManager
User->>CLI: Run describe command with --query
CLI->>ConfigManager: Retrieve configuration
ConfigManager-->>CLI: Return configuration
CLI->>YQProcessor: Evaluate query expression
YQProcessor-->>CLI: Return filtered results
CLI->>User: Display results
Possibly related PRs
Suggested Reviewers
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 (11)
website/docs/cli/commands/describe/describe-stacks.mdx (2)
47-47: Add a concrete example of the query expression.The example would be more helpful with a real-world
yqexpression. Consider adding practical examples that users can directly copy and use.-atmos describe stacks --query <yq-expression> +atmos describe stacks --query '.components | keys' +atmos describe stacks --query '.components.[].vars.namespace'
51-51: Enhance the tip with a practical example.The tip could be more helpful by including a brief example of what the filter can achieve.
-Use the `--query` flag (shorthand `-q`) to filter the output. +Use the `--query` flag (shorthand `-q`) to filter the output. For example, use `--query '.components | keys'` to list all component names.internal/exec/describe_config.go (1)
34-44: Well-structured query evaluation logic! ⚔️The implementation:
- Cleanly handles both query and non-query cases
- Maintains the original config when no query is provided
- Properly propagates evaluation errors
However, consider adding debug logging to help users understand the query evaluation process.
if query != "" { + log.Debug().Msgf("Evaluating query expression: %s", query) res, err = u.EvaluateYqExpression(atmosConfig, atmosConfig, query) if err != nil { + log.Debug().Err(err).Msg("Query evaluation failed") return err } + log.Debug().Msg("Query evaluation successful") } else { res = atmosConfig }pkg/utils/glob_utils.go (1)
24-24: Excellent cross-platform compatibility enhancement!Converting the pattern to slash format ensures consistent behavior across different operating systems. Consider adding a brief comment explaining the purpose of this conversion for future maintainers.
+ // Convert to forward slashes for cross-platform compatibility pattern = filepath.ToSlash(pattern)internal/exec/describe_workflows.go (1)
71-76: Solid query evaluation integration!The implementation aligns perfectly with the PR objective of adding query support. Consider adding a debug log when query processing is active.
if query != "" { + log.Debug().Str("query", query).Msg("Processing query expression") res, err = u.EvaluateYqExpression(atmosConfig, res, query)pkg/utils/yq_utils.go (2)
17-32: Add documentation for the logBackend struct and methods.While the implementation is correct for a no-op logger, adding documentation would help explain its purpose and usage.
+// logBackend implements a no-op logging backend for yqlib when detailed logging is not required type logBackend struct{} +// Log implements logging.Backend interface with no-op behavior func (n logBackend) Log(level logging.Level, i int, record *logging.Record) error {
34-73: Add input validation for the YQ expression.The function should validate that the YQ expression is not empty before proceeding with evaluation.
func EvaluateYqExpression(atmosConfig schema.AtmosConfiguration, data any, yq string) (any, error) { + if yq == "" { + return nil, fmt.Errorf("EvaluateYqExpression: YQ expression cannot be empty") + } + // Use the `yqlib` default (chatty) logger only when Atmos Logs Level is set to `Trace`internal/exec/describe_component.go (1)
46-73: Add documentation for the query flag behavior.While the implementation is correct, adding a comment would help explain how the query flag affects the output.
+ // If a query is provided, evaluate it against the component section using YQ + // Otherwise, return the entire component section var res any if query != "" {pkg/utils/yq_test.go (1)
11-168: Consider using table-driven tests for better maintainability.While the test coverage is excellent, restructuring to use table-driven tests would make it easier to add new test cases and maintain the existing ones.
Example structure:
func TestEvaluateYqExpression(t *testing.T) { tests := []struct { name string yq string expected any }{ { name: "get settings test value", yq: ".settings.test", expected: true, }, // Add more test cases } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { res, err := EvaluateYqExpression(atmosConfig, data, tt.yq) assert.Nil(t, err) assert.Equal(t, tt.expected, res) }) } }internal/exec/describe_affected.go (1)
241-291: Consider adding error handling for empty query results.The query evaluation logic is well-structured, but it might benefit from additional error handling for cases where the query returns no results.
Consider adding a check:
res, err := u.EvaluateYqExpression(atmosConfig, affected, query) if err != nil { return err } +if res == nil { + return fmt.Errorf("query returned no results: %s", query) +}internal/exec/describe_stacks.go (1)
116-127: Consider adding validation for query syntax.While the implementation is solid, it might benefit from validating the query syntax before evaluation.
Consider adding basic validation:
+if query != "" { + if !strings.Contains(query, ".") && !strings.Contains(query, "[") { + return fmt.Errorf("invalid query syntax: %s", query) + } res, err = u.EvaluateYqExpression(atmosConfig, finalStacksMap, query) if err != nil { return err } } else {
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (30)
LICENSE(1 hunks)atmos.yaml(1 hunks)cmd/about.go(1 hunks)cmd/describe.go(1 hunks)cmd/list_components.go(2 hunks)cmd/list_stacks.go(1 hunks)cmd/root.go(1 hunks)cmd/support.go(1 hunks)examples/quick-start-advanced/Dockerfile(1 hunks)go.mod(8 hunks)internal/exec/describe_affected.go(4 hunks)internal/exec/describe_component.go(2 hunks)internal/exec/describe_config.go(3 hunks)internal/exec/describe_dependents.go(2 hunks)internal/exec/describe_stacks.go(3 hunks)internal/exec/describe_workflows.go(2 hunks)internal/exec/utils.go(3 hunks)pkg/config/const.go(1 hunks)pkg/schema/schema.go(2 hunks)pkg/utils/file_extensions.go(1 hunks)pkg/utils/glob_utils.go(1 hunks)pkg/utils/yq_test.go(1 hunks)pkg/utils/yq_utils.go(1 hunks)website/docs/cli/commands/describe/describe-affected.mdx(2 hunks)website/docs/cli/commands/describe/describe-component.mdx(2 hunks)website/docs/cli/commands/describe/describe-config.mdx(1 hunks)website/docs/cli/commands/describe/describe-dependents.mdx(2 hunks)website/docs/cli/commands/describe/describe-stacks.mdx(2 hunks)website/docs/cli/commands/describe/describe-workflows.mdx(1 hunks)website/docs/integrations/atlantis.mdx(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- LICENSE
- cmd/list_stacks.go
- website/docs/integrations/atlantis.mdx
👮 Files not reviewed due to content moderation or server errors (3)
- website/docs/cli/commands/describe/describe-dependents.mdx
- website/docs/cli/commands/describe/describe-component.mdx
- website/docs/cli/commands/describe/describe-affected.mdx
🧰 Additional context used
📓 Learnings (1)
examples/quick-start-advanced/Dockerfile (2)
Learnt from: aknysh
PR: cloudposse/atmos#775
File: examples/quick-start-advanced/Dockerfile:9-9
Timestamp: 2024-11-12T05:52:05.088Z
Learning: It is acceptable to set `ARG ATMOS_VERSION` to a future version like `1.105.0` in `examples/quick-start-advanced/Dockerfile` if that will be the next release.
Learnt from: osterman
PR: cloudposse/atmos#801
File: examples/quick-start-advanced/Dockerfile:9-9
Timestamp: 2024-11-23T00:13:22.004Z
Learning: When updating the `ATMOS_VERSION` in Dockerfiles, the team prefers to pin to the next future version when the PR merges, even if the version is not yet released.
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Summary
🔇 Additional comments (30)
website/docs/cli/commands/describe/describe-stacks.mdx (1)
65-65: LGTM! Well-documented flag.The flag documentation is thorough and includes all necessary information:
- Clear description
- Command syntax
- Shorthand alias
- Link to detailed documentation
examples/quick-start-advanced/Dockerfile (1)
9-9: LGTM! Version bump aligns with team preferences.The update to
ATMOS_VERSION=1.140.0follows the team's practice of pinning to the next future version.Let's confirm this is the next planned version:
✅ Verification successful
Confirmed! Version 1.140.0 is indeed the next version after the latest release.
The version bump from 1.139.0 to 1.140.0 follows a clean progression, with the latest release (v1.139.0) being from 2025-01-08.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the next planned version # Check the latest released version and any pending release PRs # Get latest released version gh release list -L 1 # Check for pending release PRs gh pr list --search "release in:title" --state openLength of output: 275
go.mod (2)
39-39: LGTM! YQ dependency added for query functionality.The addition of
github.com/mikefarah/yq/v4aligns perfectly with the PR objective of adding the--queryflag to atmos describe commands.
72-72: LGTM! Supporting packages added for YQ integration.All necessary supporting packages have been added:
- envsubst for variable substitution
- participle for parsing
- utfbom for UTF handling
- orderedmap for maintaining order
- go-json and go-yaml for data processing
- copier for data copying
- gopher-lua for scripting support
These additions provide a robust foundation for the YQ integration.
Also applies to: 76-76, 123-123, 131-131, 143-144, 186-186, 212-212, 246-246
atmos.yaml (1)
18-18: LGTM! Base path configuration updated for flexibility.Setting
base_pathto an empty string allows for independent path configurations for components, stacks, and workflows. This change is well-documented and provides more flexibility in path management.Let's verify that all path configurations are properly set:
✅ Verification successful
Base path configuration verified - all paths properly set!
The empty base path at root level allows for independent path configurations in each section, while all required paths are explicitly configured in their respective sections. This follows the documented patterns and is validated by examples throughout the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if all required path configurations are explicitly set rg -A 1 "base_path:" | grep -v "^--$" | grep -v "^\s*#"Length of output: 34548
pkg/utils/file_extensions.go (1)
4-9: Clean and well-organized constants, warrior! 💪The file extension constants are well-defined and follow proper Go naming conventions. The removal of
DefaultVendoringManifestFileExtensionkeeps the codebase lean.cmd/describe.go (1)
16-17: Excellent flag implementation, commander! 🎯The
--queryflag is well-defined with:
- Clear description including usage example
- Proper shorthand
-qfor quick access- Strategic placement before command registration
internal/exec/describe_config.go (1)
19-22: Strong error handling, strategist! 🛡️Proper error checking for the query flag retrieval. This defensive programming will serve us well in battle!
cmd/about.go (1)
38-41: Fortified error handling, guardian! 🛡️The addition of error checking for
fmt.Fprintstrengthens our defenses against I/O failures. A true warrior leaves no error unchecked!cmd/support.go (1)
38-41: Strong work on error handling improvement!The addition of error checking for
fmt.Fprintoperation strengthens the robustness of the command. This is a solid defensive programming practice.internal/exec/describe_workflows.go (2)
51-54: Clean flag retrieval implementation!Proper error handling for the new query flag retrieval.
61-69: Well-structured result assignment!Clear and maintainable approach to handling different output types.
cmd/list_components.go (2)
27-33: Robust flag handling enhancement!Good improvement in error handling for flag retrieval. The error message is clear and helpful for users.
6-8: Clean import organization!The reorganization of imports improves code readability.
pkg/utils/yq_utils.go (1)
8-15: LGTM! Clean and focused imports.The imports are well-organized and include all necessary dependencies for YQ functionality and logging.
pkg/config/const.go (1)
68-69: LGTM! Well-placed constant definition.The QueryFlag constant follows the established pattern and is appropriately placed within the flags section.
internal/exec/describe_affected.go (2)
35-35: LGTM! Field addition follows the standard pattern.The
Queryfield addition toDescribeAffectedCmdArgsaligns with the PR objective of adding query support.
157-161: LGTM! Proper flag parsing implementation.The query flag parsing follows the established error handling pattern used throughout the codebase.
internal/exec/describe_dependents.go (2)
55-59: LGTM! Consistent flag parsing implementation.The query flag parsing follows the same pattern as other describe commands.
67-77: LGTM! Clean and concise query handling.The query evaluation logic is well-structured and maintains consistency with other describe commands.
internal/exec/describe_stacks.go (1)
97-101: LGTM! Consistent flag parsing.The query flag parsing maintains consistency with other describe commands.
pkg/schema/schema.go (2)
192-192: LGTM! Clean struct field addition.The Query field addition to ArgsAndFlagsInfo follows the established pattern.
257-257: LGTM! Consistent struct field addition.The Query field addition to ConfigAndStacksInfo maintains consistency with the codebase.
internal/exec/utils.go (2)
53-53: LGTM! The query flag is correctly added to commonFlags.The addition of
cfg.QueryFlagto thecommonFlagsslice follows the established pattern for flag registration.
968-980: LGTM! The query flag processing logic is well-implemented.The implementation follows the same pattern as other flags:
- Handles both standalone (
--query value) and assignment-style (--query=value) usage- Includes proper error handling for invalid flag formats
- Stores the query value in the appropriate field
website/docs/cli/commands/describe/describe-config.mdx (2)
38-38: LGTM! Clear example of query flag usage.The example follows the established pattern of command examples in the documentation.
43-46: LGTM! Well-documented flag description.The flag documentation:
- Clearly explains the purpose of the
--queryflag- Includes the
-qalias- Provides a link to the
yqdocumentation for further details- Maintains consistent table formatting
cmd/root.go (1)
113-117: LGTM! Improved error handling for SetCustomUsageFunc.The addition of error handling ensures that any issues during usage template setup are properly caught and reported, preventing silent failures.
website/docs/cli/commands/describe/describe-workflows.mdx (2)
39-39: LGTM! Clear example of query flag usage.The example follows the established pattern of command examples in the documentation.
44-48: LGTM! Well-documented flag description.The flag documentation:
- Clearly explains the purpose of the
--queryflag- Includes the
-qalias- Provides a link to the
yqdocumentation for further details- Maintains consistent table formatting with other flags
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (9)
internal/exec/describe_config.go (1)
34-44: Consider enhancing error messages for query evaluationWhile the implementation is solid, the error message from query evaluation could be more descriptive to help users debug invalid yq expressions.
Consider wrapping the error with additional context:
if query != "" { res, err = u.EvaluateYqExpression(atmosConfig, atmosConfig, query) if err != nil { - return err + return fmt.Errorf("failed to evaluate query expression '%s': %w", query, err) }pkg/utils/yq_utils.go (1)
34-73: Consider enhancing error messages and optimizing memory usage.
- Error messages could be more descriptive by including the actual data that failed to convert.
- YAML preferences could be extracted to package-level constants for reusability.
- Consider using streaming for large YAML documents to improve memory efficiency.
func EvaluateYqExpression(atmosConfig schema.AtmosConfiguration, data any, yq string) (any, error) { + // Extract YAML preferences to package-level constants + const ( + defaultIndent = 2 + defaultPrintDocSeparators = true + defaultUnwrapScalar = true + ) yaml, err := ConvertToYAML(data) if err != nil { - return nil, fmt.Errorf("EvaluateYqExpression: failed to convert data to YAML: %w", err) + return nil, fmt.Errorf("EvaluateYqExpression: failed to convert data to YAML. Data: %+v, Error: %w", data, err) } pref := yqlib.YamlPreferences{ - Indent: 2, + Indent: defaultIndent, ColorsEnabled: false, LeadingContentPreProcessing: true, - PrintDocSeparators: true, - UnwrapScalar: true, + PrintDocSeparators: defaultPrintDocSeparators, + UnwrapScalar: defaultUnwrapScalar, EvaluateTogether: false, }internal/exec/describe_component.go (1)
46-49: Consider improving error handling and code organization.
- Error messages could be more specific about what failed during query evaluation.
- Query evaluation logic could be extracted to a helper function for reuse across commands.
+// evaluateQuery evaluates a YQ expression against the given data +func evaluateQuery(query string, data any) (any, error) { + if query == "" { + return data, nil + } + + atmosConfig, err := cfg.InitCliConfig(schema.ConfigAndStacksInfo{}, true) + if err != nil { + return nil, fmt.Errorf("failed to initialize CLI config: %w", err) + } + + res, err := u.EvaluateYqExpression(atmosConfig, data, query) + if err != nil { + return nil, fmt.Errorf("failed to evaluate query '%s': %w", query, err) + } + + return res, nil +} func ExecuteDescribeComponentCmd(cmd *cobra.Command, args []string) error { // ... existing code ... query, err := flags.GetString("query") if err != nil { - return err + return fmt.Errorf("failed to get query flag: %w", err) } component := args[0] componentSection, err := ExecuteDescribeComponent(component, stack, processTemplates) if err != nil { return err } - var res any - if query != "" { - atmosConfig, err := cfg.InitCliConfig(schema.ConfigAndStacksInfo{}, true) - if err != nil { - return err - } - - res, err = u.EvaluateYqExpression(atmosConfig, componentSection, query) - if err != nil { - return err - } - } else { - res = componentSection - } + res, err := evaluateQuery(query, componentSection) + if err != nil { + return err + }Also applies to: 58-72
pkg/utils/yq_test.go (1)
11-168: Consider enhancing test coverage and organization.The test cases are comprehensive but could benefit from:
- Using table-driven tests for better organization and maintainability
- Adding edge cases (empty input, invalid expressions)
- Adding error scenario tests
Here's an example of how to refactor using table-driven tests:
func TestEvaluateYqExpression(t *testing.T) { input := ` settings: test: true mode: test # ... rest of the input ... ` atmosConfig := schema.AtmosConfiguration{ Logs: schema.Logs{ Level: "Trace", }, } data, err := UnmarshalYAML[map[string]any](input) assert.Nil(t, err) assert.NotNil(t, data) + tests := []struct { + name string + yq string + expected any + wantErr bool + }{ + { + name: "get boolean setting", + yq: ".settings.test", + expected: true, + }, + { + name: "get string setting", + yq: ".settings.mode", + expected: "test", + }, + // Add more test cases here + { + name: "invalid expression", + yq: ".[", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + res, err := EvaluateYqExpression(atmosConfig, data, tt.yq) + if tt.wantErr { + assert.Error(t, err) + return + } + assert.NoError(t, err) + assert.Equal(t, tt.expected, res) + }) + }website/docs/cli/commands/describe/describe-stacks.mdx (1)
47-47: Documentation looks good, but could be enhanced with an example query!The documentation for the new
--queryflag is clear and well-structured. Consider adding a practical example of ayqexpression to help users get started.atmos describe stacks --query <yq-expression> +# Example: Filter stacks by environment +atmos describe stacks --query '.[] | select(.environment == "prod")'Also applies to: 51-51, 65-65
website/docs/cli/commands/describe/describe-workflows.mdx (1)
39-39: Documentation is consistent and well-formatted!The documentation follows the same pattern as other commands. Consider adding a practical example query here as well.
atmos describe workflows --query <yq-expression> +# Example: Filter workflows by specific file +atmos describe workflows --query 'select(file == "networking.yaml")'Also applies to: 44-48
website/docs/cli/commands/describe/describe-dependents.mdx (1)
244-244: Documentation maintains consistency across commands!The documentation aligns well with other commands. Consider adding a practical example query for filtering dependents.
atmos describe dependents test/test-component -s tenant1-ue2-test-1 --query <yq-expression> +# Example: Filter dependents by component type +atmos describe dependents test/test-component -s tenant1-ue2-test-1 --query 'select(.component_type == "terraform")'Also applies to: 255-260
website/docs/cli/commands/describe/describe-component.mdx (1)
46-49: Examples look good but could be more descriptive!The examples demonstrate the usage of the new
--queryflag well. Consider adding comments to show expected output format.atmos describe component vpc -s plat-ue2-prod --query .vars.tags +# Returns component tags in YAML format atmos describe component vpc -s plat-ue2-prod -q .settings +# Returns component settings in YAML formatwebsite/docs/cli/commands/describe/describe-affected.mdx (1)
110-110: Example could be more specific!While the example shows the basic syntax, it would be more helpful to include a concrete example with an actual yq expression.
-atmos describe affected --query <yq-expression> +atmos describe affected --query '.[] | select(.affected == "component")' +# Lists only components affected by direct changes
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (30)
LICENSE(1 hunks)atmos.yaml(1 hunks)cmd/about.go(1 hunks)cmd/describe.go(1 hunks)cmd/list_components.go(2 hunks)cmd/list_stacks.go(1 hunks)cmd/root.go(1 hunks)cmd/support.go(1 hunks)examples/quick-start-advanced/Dockerfile(1 hunks)go.mod(8 hunks)internal/exec/describe_affected.go(4 hunks)internal/exec/describe_component.go(2 hunks)internal/exec/describe_config.go(3 hunks)internal/exec/describe_dependents.go(2 hunks)internal/exec/describe_stacks.go(3 hunks)internal/exec/describe_workflows.go(2 hunks)internal/exec/utils.go(3 hunks)pkg/config/const.go(1 hunks)pkg/schema/schema.go(2 hunks)pkg/utils/file_extensions.go(1 hunks)pkg/utils/glob_utils.go(1 hunks)pkg/utils/yq_test.go(1 hunks)pkg/utils/yq_utils.go(1 hunks)website/docs/cli/commands/describe/describe-affected.mdx(2 hunks)website/docs/cli/commands/describe/describe-component.mdx(2 hunks)website/docs/cli/commands/describe/describe-config.mdx(1 hunks)website/docs/cli/commands/describe/describe-dependents.mdx(2 hunks)website/docs/cli/commands/describe/describe-stacks.mdx(2 hunks)website/docs/cli/commands/describe/describe-workflows.mdx(1 hunks)website/docs/integrations/atlantis.mdx(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- cmd/list_stacks.go
- website/docs/integrations/atlantis.mdx
- LICENSE
🧰 Additional context used
📓 Learnings (1)
examples/quick-start-advanced/Dockerfile (2)
Learnt from: aknysh
PR: cloudposse/atmos#775
File: examples/quick-start-advanced/Dockerfile:9-9
Timestamp: 2024-11-12T05:52:05.088Z
Learning: It is acceptable to set `ARG ATMOS_VERSION` to a future version like `1.105.0` in `examples/quick-start-advanced/Dockerfile` if that will be the next release.
Learnt from: osterman
PR: cloudposse/atmos#801
File: examples/quick-start-advanced/Dockerfile:9-9
Timestamp: 2024-11-23T00:13:22.004Z
Learning: When updating the `ATMOS_VERSION` in Dockerfiles, the team prefers to pin to the next future version when the PR merges, even if the version is not yet released.
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Acceptance Tests (windows-latest, windows)
- GitHub Check: Summary
🔇 Additional comments (32)
pkg/utils/file_extensions.go (1)
4-9: Clean and well-organized constants!The file extension constants are properly maintained, and the removal of redundant constants keeps the code lean. The remaining YAML-related constants will support the new query functionality well.
cmd/describe.go (1)
16-17: Excellent implementation of the query flag!The persistent flag is well-implemented with:
- Clear description mentioning yq expressions
- Proper shorthand
-qoption- Availability across all describe subcommands
This perfectly aligns with the PR objectives.
internal/exec/describe_config.go (1)
19-22: Robust error handling for the query flag!The error handling follows the established pattern consistently.
cmd/about.go (1)
38-41: Great defensive programming!Adding error handling for fmt.Fprint improves the robustness of the command output handling. This is a solid improvement to the codebase.
cmd/support.go (1)
38-41: Excellent error handling addition, warrior!The new error check for stdout write operations strengthens the command's reliability. This is the way.
pkg/utils/glob_utils.go (1)
24-24: Path normalization strengthens cross-platform support!Converting to slash format ensures consistent glob matching across different operating systems. A worthy enhancement.
internal/exec/describe_workflows.go (4)
5-5: Clean import alias for utils package!Using
uas an alias maintains code clarity while adding the new dependency.
51-54: Strong error handling for the new query flag!The error check ensures robust command-line argument processing.
61-69: Clean result type handling!Using
anytype and clear conditional logic for different output types makes the code maintainable.
71-76: Excellent query processing implementation!The query evaluation is properly gated and error-handled. This is the core of the new functionality.
cmd/list_components.go (2)
6-8: Clean import organization!The imports are now properly grouped and organized.
27-33: Strong error handling with user-friendly output!The addition of colored error messages and proper flag error handling improves the user experience significantly.
pkg/utils/yq_utils.go (1)
17-32: LGTM! Clean and focused no-op logging implementation.The
logBackendstruct provides a clean implementation of the logging interface, effectively suppressing logs when not in trace mode.pkg/config/const.go (1)
68-68: LGTM! Well-placed constant definition.The
QueryFlagconstant follows the established pattern and is logically placed among other flag constants.cmd/root.go (1)
113-116: Great addition of error handling!The error handling for
SetCustomUsageFuncensures that any template setup failures are properly caught and reported.internal/exec/describe_affected.go (2)
35-35: LGTM! Clean implementation of the query flag.The
Queryfield is properly added to the struct and initialized through the command-line flags.Also applies to: 157-160, 179-179
241-292: Well-structured control flow for query evaluation!The implementation:
- Correctly handles the empty query case
- Properly evaluates the query using
EvaluateYqExpression- Ensures upload logic is only executed when no query is provided
This maintains a clean separation of concerns between querying and uploading functionality.
internal/exec/describe_dependents.go (2)
5-5: LGTM! Clean import and flag parsing.The utils package is properly imported and the query flag is correctly retrieved with error handling.
Also applies to: 55-58
67-78: Well-structured query evaluation logic!The implementation follows a clean pattern:
- Declares a result variable of type
any- Evaluates the query only when provided
- Falls back to the original dependents when no query is specified
internal/exec/describe_stacks.go (2)
58-58: Good refactoring for consistency!Changed to use the already retrieved
flagsvariable instead of callingcmd.Flags()again.
97-100: Clean implementation of query evaluation!The implementation follows the same consistent pattern as other describe commands:
- Proper query flag parsing with error handling
- Clear conditional logic for query evaluation
- Consistent use of
EvaluateYqExpressionAlso applies to: 116-127
pkg/schema/schema.go (1)
192-192: Clean and consistent struct field additions!The Query field is appropriately added to both info structs with consistent tagging.
Also applies to: 257-257
internal/exec/utils.go (3)
53-53: Good addition to commonFlags!The QueryFlag is properly added to the commonFlags slice, maintaining consistency with other flag declarations.
223-223: Clean field propagation!The Query field is correctly propagated from argsAndFlagsInfo to configAndStacksInfo.
968-980: Well-structured query flag handling!The implementation follows the established pattern for flag processing with proper error handling.
examples/quick-start-advanced/Dockerfile (1)
9-9: Version bump looks good!The update to ATMOS_VERSION=1.140.0 follows the team's convention of pinning to future versions.
website/docs/cli/commands/describe/describe-config.mdx (2)
38-38: Clear example addition!The example clearly demonstrates the usage of the new query flag.
43-46: Well-documented flag!The query flag documentation is comprehensive, including:
- Clear description
- Example usage
- Link to yq documentation
- Consistent table formatting
go.mod (1)
39-39: Dependencies look good, verify compatibility!The addition of
yqand its dependencies is appropriate for the new query functionality. All versions appear to be stable releases.Run this script to verify the dependencies:
Also applies to: 72-72, 76-76, 131-131, 143-144, 186-186, 212-212, 246-246
✅ Verification successful
This is a secure dependency, warrior! 🛡️
No vulnerabilities found in the GitHub Security Advisory Database for
github.com/mikefarah/yq/v4. The version v4.44.6 is battle-tested and safe to use.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any known vulnerabilities in the new dependencies # Check the yq library for any security advisories gh api graphql -f query=' { securityVulnerabilities(first: 5, ecosystem: GO, package: "github.com/mikefarah/yq") { nodes { advisory { summary severity publishedAt } vulnerableVersionRange firstPatchedVersion { identifier } } } }'Length of output: 355
atmos.yaml (1)
18-18: Configuration change provides more flexibility!Setting
base_pathto an empty string allows for independent path configurations, which is well-documented in the comments above. This change enhances flexibility in path management.website/docs/cli/commands/describe/describe-component.mdx (1)
66-66: Flag documentation is clear and well-referenced!The
--queryflag documentation provides good context with examples and links to the yq documentation.website/docs/cli/commands/describe/describe-affected.mdx (1)
214-214: Flag documentation is consistent with other commands!The
--queryflag documentation follows the same pattern as in other commands, providing clear examples and referencing yq documentation.
|
💥 This pull request now has conflicts. Could you fix it @aknysh? 🙏 |
# Conflicts: # go.mod # go.sum
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
internal/exec/describe_stacks.go (1)
116-126: Consider using a more descriptive variable name than 'res'.While the logic is solid, consider renaming
resto something more descriptive likequeryResultorstacksMapResultto better convey its purpose.- var res any + var queryResult any if query != "" { - res, err = u.EvaluateYqExpression(atmosConfig, finalStacksMap, query) + queryResult, err = u.EvaluateYqExpression(atmosConfig, finalStacksMap, query) if err != nil { return err } } else { - res = finalStacksMap + queryResult = finalStacksMap } - err = printOrWriteToFile(format, file, res) + err = printOrWriteToFile(format, file, queryResult)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
go.mod(8 hunks)internal/exec/describe_stacks.go(3 hunks)internal/exec/utils.go(3 hunks)pkg/config/const.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- pkg/config/const.go
- go.mod
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Summary
🔇 Additional comments (6)
internal/exec/describe_stacks.go (2)
58-58: LGTM! Flag retrieval changes are consistent.The changes maintain consistency in flag retrieval patterns across the function, with proper error handling.
Also applies to: 97-100
127-127: LGTM! Clean integration with existing output handling.The query evaluation is well-integrated into the existing flow, maintaining backward compatibility while adding the new query functionality.
internal/exec/utils.go (4)
53-53: LGTM! The query flag is correctly added to commonFlags.The placement ensures the flag will be properly handled and removed before passing arguments to underlying tools.
990-1001: LGTM! Query flag processing follows established patterns.The implementation correctly handles both flag formats and includes proper error handling.
245-245: LGTM! Query field is properly integrated.The field is correctly populated in the ConfigAndStacksInfo struct, maintaining consistency with other command-line arguments.
53-53: Verify query flag integration with describe commands.Let's ensure the query flag is properly integrated across all describe commands.
Run this script to verify the integration:
Also applies to: 245-245, 990-1001
✅ Verification successful
Query flag integration verified successfully across describe commands
The query flag is properly implemented and consistently integrated across all describe commands, with proper parsing and execution flow.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify query flag integration in describe commands # Test 1: Find all describe commands that should support the query flag echo "Checking describe commands for query flag support..." ast-grep --pattern 'func.*Cmd.*describe.*\(.*\).*{ $$$ }' # Test 2: Verify query flag processing in these commands echo "Checking query flag processing..." rg -A 5 'Query.*=.*argsAndFlagsInfo\.Query'Length of output: 585
Script:
#!/bin/bash # Find query flag definition and describe commands echo "=== Query flag definition ===" rg -A 3 'Query.*string.*[`"]query[`"]' echo -e "\n=== Describe command implementations ===" ast-grep --pattern 'func.*describe.*Command.*\(.*\).*{ $$$ }' echo -e "\n=== Query value usage ===" rg -A 5 'configAndStacksInfo.*Query'Length of output: 664
Script:
#!/bin/bash # Broader search for query-related code echo "=== Query flag definitions (broader search) ===" rg -i "query.*flag" -A 3 echo -e "\n=== Describe command implementations (alternative pattern) ===" rg "func.*[Dd]escribe" -A 5 echo -e "\n=== Query parameter flow ===" rg -i "query" internal/exec/utils.go -A 3Length of output: 21277
|
These changes were released in v1.140.0. |
what
--query(shorthand-q) flag to allatmos describe <subcommand>commandswhy
atmos describe <subcommand>commands using yq expressionsyqorjqbinary was required to query/filter the results ofatmos describecommands. With theyqfunctionality now embedded in Atmos, installing theyqbinary is not requiredexamples
references
Summary by CodeRabbit
Release Notes for Atmos v1.140.0
New Features
--queryflag across multipledescribecommands to filter results using YQ expressions.Improvements
Dependency Updates
yq/v4and several indirect dependencies.Documentation
Chores