8000 Add pager to atmos describe dependents command by samtholiya · Pull Request #1295 · cloudposse/atmos · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add pager to atmos describe dependents command #1295

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

Open
wants to merge 153 commits into
base: main
Choose a base branch
from

Conversation

samtholiya
Copy link
Collaborator
@samtholiya samtholiya commented Jun 11, 2025

what

  • pager for describe dependents
    image

why

  • helps devops view the dependents better in case of huge dependents list / content

references

Summary by CodeRabbit

  • New Features

    • Added support for filtering "describe dependents" command output using a new --query flag.
    • Output format validation now enforces "json" or "yaml" formats, with "json" as the default.
  • Bug Fixes

    • Improved error handling and argument validation for the "describe dependents" command.
  • Refactor

    • Restructured the "describe dependents" command for greater modularity and testability.
  • Tests

    • Introduced comprehensive unit tests for the "describe dependents" command and its execution logic.

samtholiya and others added 30 commits April 14, 2025 23:21
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link
codecov bot commented Jun 11, 2025

Codecov Report

Attention: Patch coverage is 95.83333% with 3 lines in your changes missing coverage. Please review.

Project coverage is 51.06%. Comparing base (a9990e9) to head (502a01e).

Files with missing lines Patch % Lines
cmd/describe_dependents.go 90.62% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1295      +/-   ##
==========================================
+ Coverage   50.46%   51.06%   +0.59%     
==========================================
  Files         237      238       +1     
  Lines       25777    25799      +22     
==========================================
+ Hits        13008    13173     +165     
+ Misses      11151    10989     -162     
- Partials     1618     1637      +19     
Flag Coverage Δ
unittests 51.06% <95.83%> (+0.59%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@settings settings bot removed the stacked label Jun 12, 2025
@mergify mergify bot added the stacked label Jun 12, 2025
@settings settings bot removed the stacked label Jun 13, 2025
@mergify mergify bot added the stacked Stacked label Jun 13, 2025
Base automatically changed from feature/dev-3251-add-pager-to-atmos-describe-workflow-command to main June 13, 2025 19:55
@mergify mergify bot removed the stacked Stacked label Jun 13, 2025
@github-actions github-actions bot added the size/l Large size PR label Jun 13, 2025
@samtholiya samtholiya added the minor New features that do not break anything label Jun 13, 2025
@github-actions github-actions bot added size/m Medium size PR and removed size/l Large size PR labels Jun 13, 2025
@github-actions github-actions bot added size/l Large size PR and removed size/m Medium size PR labels Jun 13, 2025
@samtholiya samtholiya marked this pull request as ready for review June 18, 2025 21:15
@samtholiya samtholiya requested a review from a team as a code owner June 18, 2025 21:15
Copy link
Contributor
coderabbitai bot 6D47 commented Jun 18, 2025
📝 Walkthrough

Walkthrough

The "describe dependents" command was refactored for modularity and testability. Execution logic is now handled by an interface-driven executor, with dependencies injected for easier testing. A new query flag was added for JMESPath filtering. Paging support and improved flag validation were implemented. Comprehensive unit tests and mocks were introduced.

Changes

File(s) Change Summary
cmd/describe_dependents.go Refactored command to use dependency injection; added query flag; improved argument/flag handling.
cmd/describe_dependents_test.go Added unit tests for command execution and flag parsing.
internal/exec/describe_dependents.go Refactored execution logic into interface; added paging, query support, and modular output handling.
internal/exec/describe_dependents_test.go Added unit tests for executor covering queries, errors, formats, and paging.
internal/exec/mock_describe_dependents.go Added GoMock-generated mock for executor interface.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant CLI
    participant Cmd as describeDependentsCmd
    participant Executor as DescribeDependentsExec
    participant Pager
    participant Output

    User->>CLI: Run "describe dependents COMPONENT [flags]"
    CLI->>Cmd: Parse args, validate config, set flags
    Cmd->>Executor: Execute(props)
    Executor->>Executor: Fetch dependents
    Executor->>Executor: (Optional) Apply query filter
    Executor->>Pager: If output large, use pager
    Pager->>Output: Display results
    Executor->>Cmd: Return status
    Cmd->>CLI: Exit
Loading

Assessment against linked issues

Objective Addressed Explanation
Add pager to atmos describe dependents Command (DEV-3431)

Suggested reviewers

  • osterman
  • aknysh

📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 80f5f5c and fb4ec5f.

📒 Files selected for processing (5)
  • cmd/describe_dependents.go (2 hunks)
  • cmd/describe_dependents_test.go (1 hunks)
  • internal/exec/describe_dependents.go (1 hunks)
  • internal/exec/describe_dependents_test.go (1 hunks)
  • internal/exec/mock_describe_dependents.go (1 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
cmd/describe_dependents.go

[warning] 48-49: cmd/describe_dependents.go#L48-L49
Added lines #L48 - L49 were not covered by tests

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Summary
🔇 Additional comments (15)
internal/exec/mock_describe_dependents.go (1)

1-49: Generated mock looks good.

The GoMock-generated mock follows standard patterns and provides proper mocking capabilities for the DescribeDependentsExec interface. This enables effective unit testing of the command layer.

cmd/describe_dependents_test.go (2)

14-29: Solid test setup with dependency injection.

The test effectively uses the dependency injection pattern to mock all external dependencies, ensuring isolated testing of the command logic.


31-83: Comprehensive flag validation testing.

The table-driven tests cover all the important flag scenarios including valid formats, defaults, and error cases. This ensures the flag parsing logic works correctly.

internal/exec/describe_dependents_test.go (4)

13-23: Good constructor testing.

The test verifies that the constructor properly initializes all dependencies, ensuring the executor is created correctly.


25-120: Excellent test coverage for success scenarios.

Both query and non-query execution paths are thoroughly tested with proper mock setup and assertions.


122-192: Comprehensive error handling tests.

The tests cover both executor failure and query evaluation failure scenarios, ensuring robust error handling.


194-282: Thorough edge case and format testing.

Tests cover empty results and different output formats/destinations, providing comprehensive validation of the executor behavior.

internal/exec/describe_dependents.go (3)

18-29: Clean interface design.

The DescribeDependentsExec interface provides a clean abstraction with the DescribeDependentsExecProps struct encapsulating all parameters nicely.


31-56: Excellent dependency injection pattern.

The struct design with injected dependencies makes this highly testable while maintaining clean separation of concerns.


58-90: Well-implemented execute method.

The method cleanly handles both query and non-query scenarios, integrating the new filtering functionality seamlessly with the existing pager support.

cmd/describe_dependents.go (5)

13-29: Solid refactoring with dependency injection.

The command now properly separates CLI concerns from business logic using dependency injection, making it much more testable and maintainable.


31-55: Clean command execution flow.

The execution flow is well-structured with proper error handling at each step and clean separation of responsibilities.


57-73: Good flag validation logic.

The flag parsing and validation is handled cleanly with appropriate defaults and error checking for invalid formats.


81-81: New query flag properly integrated.

The JMESPath query flag is correctly added and will enable users to filter the dependents output as requested in the PR objectives.


47-50: Add test coverage for pager flag handling.

Static analysis indicates these lines aren't covered by tests. Consider adding a test case that exercises the pager flag functionality.

#!/bin/bash
# Description: Verify if there are existing tests for pager flag handling
# Expected: Find test cases that set the pager flag

rg -A 10 -B 5 'pager.*flag|flag.*pager' --type go
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate Unit Tests
  • Create PR with Unit Tests
  • Post Copyable Unit Tests in Comment
  • Commit Unit Tests in branch feature/dev-3431-add-pager-to-atmos-describe-dependents-command

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai auto-generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary or @auto-summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai or @auto-title anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

8623
@mergify mergify bot removed the triage Needs triage label Jun 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor New features that do not break anything size/l Large size PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0