Add namespace support to output commands #67
Conversation
- Update add_input to support @namespace/name format - Add namespace support to get_output_types - Rename get_output_details to get_output_type_details with namespace support - Add comprehensive test coverage for all changes
|
""" WalkthroughThe changes introduce namespace-aware output type handling in the CLI, requiring output types to be specified in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant ControlPlaneAPI
User->>CLI: Run get_output_type_details --output-type @namespace/name
CLI->>CLI: Validate output type format
CLI->>CLI: Check login credentials
alt Not logged in
CLI-->>User: Error: Please login
else Logged in
CLI->>ControlPlaneAPI: GET /cc-ui/v1/tf-outputs (with token)
ControlPlaneAPI-->>CLI: List of output types
CLI->>CLI: Find output type by namespace and name
alt Output type found
CLI-->>User: Display output type details, properties, lookup tree
else Not found
CLI-->>User: Error: Output type not found
end
end
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed 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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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 (7)
ftf_cli/commands/add_input.py (2)
20-38: Consider extracting shared validation function to utils.The
parse_namespace_and_namefunction is identical to the one inget_output_type_details.py. To follow DRY principles, consider moving this function toftf_cli/utils.pyfor reuse across commands.If you'd like me to help create the shared utility function, I can generate the refactor for you.
149-150: Apply performance improvement from static analysis.The static analysis tool correctly identifies an optimization opportunity.
Apply this diff to improve performance:
- available_output_types = [f'{namespace}/{name}' for namespace, name in registered_outputs.keys()] + available_output_types = [f'{namespace}/{name}' for namespace, name in registered_outputs]tests/commands/test_get_output_type_details.py (1)
2-2: Remove unused import.The
jsonmodule is imported but never used in the test file.-import jsonftf_cli/commands/get_output_type_details.py (4)
2-2: Remove unused import.The
osmodule is imported but never used in this file.-import os
77-77: Remove unnecessary.keys()call.Using
key in dictis more efficient thankey in dict.keys().- available_outputs = [f'{ns}/{nm}' for ns, nm in registered_outputs.keys()] + available_outputs = [f'{ns}/{nm}' for ns, nm in registered_outputs]
94-94: Remove unnecessary f-string prefixes.These strings don't contain any placeholders, so the
fprefix is unnecessary.- click.echo(f"\n--- Properties ---") + click.echo("\n--- Properties ---")- click.echo(f"\n--- Properties ---") + click.echo("\n--- Properties ---")- click.echo(f"\n--- Lookup Tree ---") + click.echo("\n--- Lookup Tree ---")- click.echo(f"\n--- Lookup Tree ---") + click.echo("\n--- Lookup Tree ---")Also applies to: 98-98, 103-103, 110-110
118-122: Improve exception handling.Use
raise ... from eto preserve the original exception context for better debugging.except Exception as e: traceback.print_exc() raise click.UsageError( f"❌ An error occurred while getting output details: {e}" - ) + ) from e
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
ftf_cli/cli.py(2 hunks)ftf_cli/commands/__init__.py(2 hunks)ftf_cli/commands/add_input.py(6 hunks)ftf_cli/commands/get_output_details.py(0 hunks)ftf_cli/commands/get_output_type_details.py(1 hunks)ftf_cli/commands/get_output_types.py(2 hunks)tests/commands/test_add_input.py(13 hunks)tests/commands/test_get_output_type_details.py(1 hunks)tests/commands/test_get_output_types.py(1 hunks)
💤 Files with no reviewable changes (1)
- ftf_cli/commands/get_output_details.py
🧰 Additional context used
🧠 Learnings (2)
tests/commands/test_get_output_types.py (1)
Learnt from: anujhydrabadi
PR: Facets-cloud/module-development-cli#59
File: tests/commands/test_preview_module.py:91-99
Timestamp: 2025-05-30T10:34:53.821Z
Learning: When patching imported functions in Python unittest.mock, patch where the object is looked up by the system under test, not where it's defined. If there's a namespace collision (like Click commands exported in __init__.py), use patch.object() with direct module import to bypass the collision.
tests/commands/test_get_output_type_details.py (1)
Learnt from: anujhydrabadi
PR: Facets-cloud/module-development-cli#59
File: tests/commands/test_preview_module.py:91-99
Timestamp: 2025-05-30T10:34:53.821Z
Learning: When patching imported functions in Python unittest.mock, patch where the object is looked up by the system under test, not where it's defined. If there's a namespace collision (like Click commands exported in __init__.py), use patch.object() with direct module import to bypass the collision.
🧬 Code Graph Analysis (5)
ftf_cli/cli.py (1)
ftf_cli/commands/get_output_type_details.py (1)
get_output_type_details(46-122)
ftf_cli/commands/__init__.py (1)
ftf_cli/commands/get_output_type_details.py (1)
get_output_type_details(46-122)
tests/commands/test_get_output_types.py (3)
ftf_cli/commands/get_output_types.py (1)
get_output_types(15-54)tests/commands/test_add_input.py (2)
runner(17-18)mock_credentials(21-26)tests/commands/test_get_output_type_details.py (2)
runner(13-14)mock_credentials(17-22)
ftf_cli/commands/add_input.py (1)
ftf_cli/commands/get_output_type_details.py (1)
parse_namespace_and_name(11-29)
ftf_cli/commands/get_output_type_details.py (2)
ftf_cli/utils.py (2)
is_logged_in(504-537)get_profile_with_priority(486-501)ftf_cli/commands/add_input.py (1)
parse_namespace_and_name(20-38)
🪛 Ruff (0.11.9)
ftf_cli/commands/add_input.py
150-150: Use key in dict instead of key in dict.keys()
Remove .keys()
(SIM118)
tests/commands/test_get_output_type_details.py
2-2: json imported but unused
Remove unused import: json
(F401)
ftf_cli/commands/get_output_type_details.py
2-2: os imported but unused
Remove unused import: os
(F401)
77-77: Use key in dict instead of key in dict.keys()
Remove .keys()
(SIM118)
94-94: f-string without any placeholders
Remove extraneous f prefix
(F541)
98-98: f-string without any placeholders
Remove extraneous f prefix
(F541)
103-103: f-string without any placeholders
Remove extraneous f prefix
(F541)
110-110: f-string without any placeholders
Remove extraneous f prefix
(F541)
120-122: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🪛 Flake8 (7.2.0)
tests/commands/test_get_output_type_details.py
[error] 2-2: 'json' imported but unused
(F401)
ftf_cli/commands/get_output_type_details.py
[error] 2-2: 'os' imported but unused
(F401)
[error] 94-94: f-string is missing placeholders
(F541)
[error] 98-98: f-string is missing placeholders
(F541)
[error] 103-103: f-string is missing placeholders
(F541)
[error] 110-110: f-string is missing placeholders
(F541)
🔇 Additional comments (16)
ftf_cli/cli.py (1)
11-11: LGTM! Clean refactoring to new command structure.The import and command registration have been correctly updated to use the new
get_output_type_detailsfunction, maintaining consistency across the CLI structure.Also applies to: 31-31
ftf_cli/commands/__init__.py (1)
10-10: LGTM! Package exports correctly updated.The import and
__all__list have been consistently updated to reflect the new command structure, maintaining proper package organization.Also applies to: 26-26
ftf_cli/commands/get_output_types.py (1)
39-41: LGTM! Correct namespace-aware output formatting.The implementation properly combines namespace and name with a sensible default fallback, correctly supporting the new
@namespace/nameformat requirement.tests/commands/test_get_output_types.py (1)
1-161: Excellent comprehensive test coverage.The test suite thoroughly covers all scenarios including:
- Successful output type retrieval with namespace formatting
- Empty response handling
- Default namespace fallback behavior
- Authentication error cases
- API failure scenarios
The use of pytest fixtures and proper mocking demonstrates good testing practices.
ftf_cli/commands/add_input.py (4)
75-75: LGTM! Clear help text specifying required format.The help text clearly communicates the expected
@namespace/nameformat with examples, improving user experience.
129-130: LGTM! Good practice with early validation.Validating the output type format early in the function helps provide immediate feedback to users and prevents processing invalid input.
153-158: LGTM! Correct namespace-aware validation logic.The validation correctly uses namespace/name tuples for lookup and provides helpful error messages with available options.
169-198: LGTM! Robust property structure handling.The dual structure handling (direct vs nested properties) with proper error handling and fallbacks ensures compatibility with different API response formats while providing helpful warnings to users.
tests/commands/test_get_output_type_details.py (2)
9-67: Excellent test structure and fixtures.The test class and fixtures are well-designed with comprehensive sample data that covers both default and custom namespaces. The API response fixture properly represents the expected data structure with namespace-qualified output types.
68-262: Comprehensive test coverage for all scenarios.The test methods provide excellent coverage of:
- Successful output type retrieval with proper format validation
- Custom namespace handling
- Error cases (not found, invalid format, not logged in, API errors)
- Edge cases (missing properties, missing lookup tree)
The test assertions are thorough and verify both exit codes and output content appropriately.
ftf_cli/commands/get_output_type_details.py (1)
46-117: Command logic is well-implemented.The core functionality correctly:
- Validates the namespace/name format
- Authenticates users and fetches credentials
- Makes appropriate API calls
- Handles response processing and error cases
- Formats output with proper structure
The namespace-aware lookup and error messaging provide good user experience.
tests/commands/test_add_input.py (5)
64-64: Well-structured test data updates for namespace support.The sample API response properly includes namespace fields and adds a custom namespace example (
@anuj/sqs) with direct properties structure. This provides good test coverage for both default and custom namespaces.Also applies to: 102-102, 117-128
175-175: Consistent migration to namespace-qualified format.All test invocations have been properly updated to use the
@namespace/nameformat (e.g.,@outputs/database) instead of the old simple names. This ensures comprehensive testing of the new format requirements.Also applies to: 220-220, 243-243, 273-273, 318-318, 359-359, 421-421, 452-452
459-518: Excellent format validation test coverage.The new tests comprehensively validate the namespace format requirements:
- Missing
@prefix is properly rejected- Missing slash separator is properly rejected
- Error messages provide clear guidance on expected format
This ensures robust input validation for the namespace-aware functionality.
519-559: Good coverage for custom namespace functionality.This test confirms that custom namespaces (like
@anuj/sqs) work correctly, ensuring the namespace support isn't limited to just the default@outputsnamespace.
560-605: Valuable edge case testing for direct properties structure.This test ensures that outputs with direct
attributes/interfacesstructure (without nested"type": "object"wrappers) are handled correctly without warnings. This covers an important structural variation in the API responses.
- Handle both JSON Schema format and direct properties format - Fix issue where @anuj/sqs type outputs generated 'any' instead of proper types - Add support for empty interfaces/attributes objects
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
ftf_cli/commands/get_output_type_details.py (1)
95-99: Improve exception chaining for better debugging.Consider using
raise ... from eto preserve the exception chain, which helps with debugging by showing the original cause of the error.except Exception as e: traceback.print_exc() raise click.UsageError( f"❌ An error occurred while getting output details: {e}" - ) + ) from e
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
ftf_cli/commands/add_input.py(6 hunks)ftf_cli/commands/get_output_type_details.py(1 hunks)ftf_cli/utils.py(5 hunks)tests/commands/test_add_input.py(13 hunks)tests/commands/test_get_output_type_details.py(1 hunks)tests/commands/test_get_output_types.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/commands/test_get_output_types.py
- tests/commands/test_add_input.py
- tests/commands/test_get_output_type_details.py
🧰 Additional context used
🧬 Code Graph Analysis (2)
ftf_cli/commands/add_input.py (1)
ftf_cli/utils.py (1)
parse_namespace_and_name(21-39)
ftf_cli/commands/get_output_type_details.py (1)
ftf_cli/utils.py (3)
is_logged_in(525-558)get_profile_with_priority(507-522)parse_namespace_and_name(21-39)
🪛 Ruff (0.11.9)
ftf_cli/commands/add_input.py
130-130: Use key in dict instead of key in dict.keys()
Remove .keys()
(SIM118)
ftf_cli/commands/get_output_type_details.py
54-54: Use key in dict instead of key in dict.keys()
Remove .keys()
(SIM118)
97-99: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🔇 Additional comments (8)
ftf_cli/utils.py (2)
21-39: LGTM! Well-implemented utility function.The
parse_namespace_and_namefunction is well-designed with clear documentation, appropriate error handling, and helpful error messages that include examples. This implementation successfully addresses the DRY principle by centralizing the namespace/name parsing logic that was previously duplicated across multiple files.
894-930: LGTM! Improved handling of property structures.The new logic correctly handles direct property dictionaries (like
{"queue_arn": {"type": "string"}}) while maintaining backward compatibility with existing JSON Schema structures. The improved fallback to return"object({})"instead of"any"provides better type safety for empty or unknown structures.ftf_cli/commands/add_input.py (3)
17-17: LGTM! Proper use of centralized utility function.Good adoption of the centralized
parse_namespace_and_namefunction from the utils module, following DRY principles.
109-110: LGTM! Good practice of early validation.Validating the output type format early in the function provides immediate feedback and follows the fail-fast principle, avoiding unnecessary API calls when the format is invalid.
149-171: LGTM! Robust property extraction logic.The updated logic correctly handles both direct and nested property structures, making the code more resilient to different API response formats. The fallback mechanism ensures graceful degradation when expected structures are not found.
ftf_cli/commands/get_output_type_details.py (3)
1-22: LGTM! Well-structured command implementation.The command follows established CLI patterns with proper imports and option definitions. Good use of the centralized utility functions and consistent help text formatting.
26-35: LGTM! Proper validation and authentication flow.Good practice of validating the output type format before proceeding with authentication and API calls. Clear error messages provide helpful guidance to users.
59-89: LGTM! Well-structured output display with good error handling.The display logic is well-organized with clear sections and appropriate fallbacks. The JSON error handling for the lookup tree is particularly good, ensuring the command doesn't fail on malformed data while providing useful feedback.
Summary by CodeRabbit
New Features
@namespace/name).Bug Fixes
Tests
Refactor
@namespace/nameformat.