-
-
Notifications
You must be signed in to change notification settings - Fork 111
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: create atmos list
command for listing stacks and components
#797
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces a new command structure in the Atmos CLI, specifically adding a Changes
Possibly related PRs
Suggested reviewers
Warning Rate limit exceeded@RoseSecurity has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 33 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 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
|
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: 3
🧹 Outside diff range and nitpick comments (9)
cmd/list.go (1)
7-13
: Enhance command documentation and configurationWhile the basic structure is solid, consider these improvements to enhance user experience:
Add more detailed documentation in the
Long
description, including:
- Available subcommands (stacks, components)
- Example usage
- Expected output format
Consider adding these Cobra fields:
Example
field for usage examplesValidArgs
for subcommand validationHere's a suggested enhancement:
var listCmd = &cobra.Command{ Use: "list", Short: "Execute 'list' commands", - Long: `This command lists stacks and components`, + Long: `This command lists stacks and components in your Atmos configuration. + +Available Commands: + components List available components + stacks List available stacks + +Usage: + atmos list [command]`, + Example: ` # List all stacks + atmos list stacks + + # List components for a specific stack + atmos list components -s staging`, + ValidArgs: []string{"stacks", "components"}, FParseErrWhitelist: struct{ UnknownFlags bool }{UnknownFlags: false}, }website/docs/cli/commands/list/list-stacks.mdx (3)
6-6
: Consider aligning the description with the purpose note for consistency.The description in the metadata is more detailed than the purpose note. Consider making them consistent to avoid confusion.
description: Use this command to list all Stack configurations or a stack of a specified component. ... - Use this command to list Atmos stacks. + Use this command to list all Stack configurations or a stack of a specified component.Also applies to: 12-12
15-15
: Add alt text for accessibility.The Screengrab component should include alt text to improve accessibility for screen readers.
- <Screengrab title="atmos list stacks --help" slug="atmos-list-stacks--help" /> + <Screengrab + title="atmos list stacks --help" + slug="atmos-list-stacks--help" + alt="Help output for atmos list stacks command showing available options and usage" + />
17-33
: Add performance benefits information.Given that the PR objectives mention significant performance improvements (0.741s vs 8.131s), consider adding this information to highlight the command's efficiency.
## Usage Execute the `list stacks` command like this: + :::info Performance + This command executes significantly faster than the previous custom command implementation, + completing in approximately 0.741 seconds compared to the previous 8.131 seconds. + ::: ```shellcmd/list_stacks.go (3)
17-24
: Consider enhancing the command examples.While the command structure is solid, the examples could be more realistic and helpful.
Consider updating the example to include actual component names and expected output:
- Example: "atmos list stacks\n" + - "atmos list stacks -c <component>", + Example: "# List all stacks\n" + + "atmos list stacks\n\n" + + "# List stacks for a specific component\n" + + "atmos list stacks -c eks\n" + + "atmos list stacks -c vpc",
76-80
: Consider adding component flag validation.While the flag setup is correct, adding validation would improve user experience.
Consider adding flag validation:
func init() { listStacksCmd.DisableFlagParsing = false listStacksCmd.PersistentFlags().StringP("component", "c", "", "atmos list stacks -c <component>") + listStacksCmd.MarkPersistentFlagRequired("component") // If component is required + // Or add custom validation + listStacksCmd.PreRunE = func(cmd *cobra.Command, args []string) error { + component, _ := cmd.Flags().GetString("component") + if component != "" && !isValidComponent(component) { + return fmt.Errorf("invalid component name: %s", component) + } + return nil + } listCmd.AddCommand(listStacksCmd) }
25-42
: Consider adding performance metrics.The PR objectives mention significant performance improvements (0.741s vs 8.131s), but there's no instrumentation to verify these claims.
Consider adding execution time tracking:
Run: func(cmd *cobra.Command, args []string) { + start := time.Now() + defer func() { + duration := time.Since(start) + if os.Getenv("ATMOS_DEBUG") == "true" { + fmt.Printf("Command execution time: %v\n", duration) + } + }() // Check Atmos configuration checkAtmosConfig()cmd/list_components.go (2)
17-24
: Consider enhancing command examples with realistic component names.The command documentation is clear, but could be more helpful with concrete examples:
- Example: "atmos list components\n" + - "atmos list components -s <stack>", + Example: "atmos list components\n" + + "atmos list components -s prod-us-west-2\n" + + "atmos list components -s staging-us-east-1",
33-34
: Enhance error messages with troubleshooting guidance.The error messages could be more helpful by including:
- Potential causes
- Troubleshooting steps
- Links to documentation
-u.PrintMessageInColor(fmt.Sprintf("Error initializing CLI config: %v", err), color.New(color.FgRed)) +u.PrintMessageInColor(fmt.Sprintf( + "Error initializing CLI config: %v\nPlease ensure:\n"+ + "- atmos.yaml exists in the current directory\n"+ + "- You have necessary permissions\n"+ + "For more details, see: https://atmos.tools/cli/configuration", + err), color.New(color.FgRed))Also applies to: 39-40
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (20)
atmos.yaml
(0 hunks)cmd/list.go
(1 hunks)cmd/list_components.go
(1 hunks)cmd/list_stacks.go
(1 hunks)examples/quick-start-advanced/atmos.yaml
(0 hunks)examples/quick-start-advanced/rootfs/usr/local/etc/atmos/atmos.yaml
(0 hunks)examples/tests/atmos.yaml
(0 hunks)examples/tests/rootfs/usr/local/etc/atmos/atmos.yaml
(0 hunks)pkg/aws/atmos.yaml
(0 hunks)pkg/generate/atmos.yaml
(0 hunks)pkg/validate/atmos.yaml
(0 hunks)website/docs/cli/commands/list/_category_.json
(1 hunks)website/docs/cli/commands/list/list-components.mdx
(1 hunks)website/docs/cli/commands/list/list-stacks.mdx
(1 hunks)website/docs/cli/commands/list/usage.mdx
(1 hunks)website/docs/cli/commands/pro/_category_.json
(1 hunks)website/docs/cli/commands/terraform/_category_.json
(1 hunks)website/docs/cli/commands/validate/_category_.json
(1 hunks)website/docs/cli/commands/vendor/_category_.json
(1 hunks)website/docs/core-concepts/custom-commands/custom-commands.mdx
(0 hunks)
💤 Files with no reviewable changes (9)
- atmos.yaml
- examples/quick-start-advanced/atmos.yaml
- examples/quick-start-advanced/rootfs/usr/local/etc/atmos/atmos.yaml
- examples/tests/atmos.yaml
- examples/tests/rootfs/usr/local/etc/atmos/atmos.yaml
- pkg/aws/atmos.yaml
- pkg/generate/atmos.yaml
- pkg/validate/atmos.yaml
- website/docs/core-concepts/custom-commands/custom-commands.mdx
✅ Files skipped from review due to trivial changes (7)
- website/docs/cli/commands/list/category.json
- website/docs/cli/commands/list/list-components.mdx
- website/docs/cli/commands/list/usage.mdx
- website/docs/cli/commands/pro/category.json
- website/docs/cli/commands/terraform/category.json
- website/docs/cli/commands/validate/category.json
- website/docs/cli/commands/vendor/category.json
🔇 Additional comments (7)
cmd/list.go (3)
1-6
: LGTM! Clean and focused imports.
The package declaration and imports are minimal and appropriate for the command implementation.
15-17
: LGTM! Clean command initialization.
The initialization follows Cobra's best practices for command registration.
1-17
: Verify command integration and performance claims
The PR claims performance improvements from 8.131s to 0.741s. Let's verify the command structure and look for any performance-related code.
website/docs/cli/commands/list/list-stacks.mdx (1)
42-46
: Verify all available command flags are documented.
The tip mentions additional options available through --help, but only the --component flag is documented. Please verify if there are other flags that should be included in this table.
✅ Verification successful
Documentation accurately reflects all available flags
After examining the command implementation in cmd/list_stacks.go
, I can confirm that --component
(with alias -c
) is indeed the only flag available for the list stacks
command. The documentation in the MDX file correctly reflects all available command flags.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any additional flags in the codebase
# Search for flag definitions in the list stacks command implementation
rg -A 5 'flag\.' cmd/list_stacks.go
Length of output: 37
Script:
#!/bin/bash
# Let's try searching in the entire codebase for list-stacks command flags
rg -i "list-stacks.*flag" -A 5
# Also search for any Go files that might contain the command implementation
fd "list.*stacks.*\.go$"
# Search for any CLI documentation files related to list-stacks
fd "list.*stacks.*\.mdx?$"
Length of output: 154
Script:
#!/bin/bash
# Let's examine the list_stacks.go implementation
cat cmd/list_stacks.go
# Also check if there's any cobra command definition that might show flags
rg -A 5 "cobra\.Command.*list-stacks"
Length of output: 2486
cmd/list_stacks.go (1)
3-15
: LGTM! Clean and well-organized imports.
The imports are properly structured and all are being utilized effectively in the code.
cmd/list_components.go (2)
1-16
: LGTM! Clean and purposeful imports.
The package structure follows Go conventions, and imports are well-organized with meaningful aliases.
81-84
: LGTM! Clean command initialization.
The command initialization follows Cobra best practices with clear flag description.
Updating this issue based on our conversation in slack. Can we make the output configurable in E.g.
This would output something like:
Ideally, we would use lipgloss tables And degrade to plain text, if no TTY attached (e.g. for pipelining of commands, into things like grep) |
I don't have the bandwidth to implement the discussed functionality right now, but I'm happy to leave this PR open for further iteration as needed! In the future, when I have more time, I'd be glad to work on adding the pretty-printing feature |
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: 5
🧹 Outside diff range and nitpick comments (7)
pkg/list/list_stacks.go (2)
8-8
: Consider using native Go implementation instead of external dependency.The
lo
package is only used for theKeys()
function. A native implementation would reduce dependencies and could be as simple as:func getMapKeys(m map[string]any) []string { keys := make([]string, 0, len(m)) for k := range m { keys = append(keys, k) } return keys }
19-19
: Consider making the component path configurable.The hardcoded "terraform" key suggests this function might not work for other component types. Consider making this configurable through the
atmos.yaml
configuration.pkg/list/list_components.go (2)
11-13
: Enhance function documentation for better clarity.Consider expanding the documentation to include:
- Parameter descriptions
- Return value descriptions
- Example usage
-// FilterAndListComponents filters and lists components based on the given stack +// FilterAndListComponents filters and lists components based on the given stack. +// Parameters: +// - stackFlag: Optional stack identifier to filter components +// - stacksMap: Map of stacks containing component configurations +// Returns: +// - string: Newline-separated list of components or status message +// - error: Error if specified stack not found or processing fails
41-49
: LGTM! Consider adding error context for empty results.The results processing is clean and effective. Consider distinguishing between "no components found" due to filtering vs. invalid data structure.
if len(components) == 0 { - return "No components found", nil + if stackFlag != "" { + return fmt.Sprintf("No components found in stack '%s'", stackFlag), nil + } + return "No components found in any stacks", nil }cmd/list_components.go (3)
15-22
: Enhance command documentation with more examples.Consider adding more comprehensive examples to demonstrate different use cases:
- Filtering by multiple stacks
- Example output format
- Common usage patterns
Example: "atmos list components\n" + - "atmos list components -s <stack>", + "atmos list components -s <stack>\n" + + "# List components for a specific stack\n" + + "atmos list components -s prod-ue2\n" + + "# Example output:\n" + + "# - eks-cluster\n" + + "# - vpc",
26-26
: Handle potential flag parsing error.The error from
GetString
is silently ignored. While unlikely to fail, it's better to handle all potential errors for robustness.- stackFlag, _ := cmd.Flags().GetString("stack") + stackFlag, err := cmd.Flags().GetString("stack") + if err != nil { + u.PrintMessageInColor(fmt.Sprintf("Error getting stack flag: %v", err), color.New(color.FgRed)) + return + }
41-47
: Consider making output format configurable.As suggested in PR comments by osterman, the output format should be configurable through
atmos.yaml
. This would allow users to:
- Specify which columns to display (Stack, Tenant, Environment, File)
- Use lipgloss tables for terminal output
- Fallback to plain text for non-TTY usage
Would you like help implementing this enhancement?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
cmd/list_components.go
(1 hunks)cmd/list_stacks.go
(1 hunks)pkg/list/list_components.go
(1 hunks)pkg/list/list_stacks.go
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- cmd/list_stacks.go
🔇 Additional comments (3)
pkg/list/list_stacks.go (2)
28-33
: LGTM: Clean error handling and output formatting.
The error handling for empty results and the output formatting with newlines is well implemented.
35-39
: LGTM: Efficient implementation of listing all stacks.
The implementation efficiently handles the case when no component filter is specified, using the lo.Keys
function and proper sorting.
pkg/list/list_components.go (1)
1-9
: LGTM! Clean and focused imports.
The imports are well-organized and all are being utilized effectively in the implementation.
Let's merge as soon as it's working, and we'll use this as the first pass. Thanks again! |
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.
thanks @RoseSecurity
These changes were released in v1.110.0. |
what
atmos list
commands for listing stacks and componentslist
commands into Atmosatmos.yaml
references to custom list commandwhy
testing
quick-start-advanced
❯ ./atmos list components -s invalid-stack Error: stack 'invalid-stack' not found
references
Summary by CodeRabbit
Release Notes
New Features
list
command with subcommands for listing Atmos stacks and components.Documentation
list
,list stacks
, andlist components
commands, including usage examples and flags.Bug Fixes