Add --query (shorthand -q) flag to all atmos describe <subcommand> commands#920
Add --query (shorthand -q) flag to all atmos describe <subcommand> commands#920
--query (shorthand -q) flag to all atmos describe <subcommand> commands#920Conversation
# Conflicts: # cmd/terraform.go # go.mod # go.sum
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