-
Notifications
You must be signed in to change notification settings - Fork 24
feat(cli): Add CLI commands: airbyte-cdk manifest validate
, migrate
, and normalize
#616
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
base: main
Are you sure you want to change the base?
feat(cli): Add CLI commands: airbyte-cdk manifest validate
, migrate
, and normalize
#616
Conversation
- Add validate command to check manifest against declarative component schema - Add migrate command to apply migrations and update to latest CDK version - Both commands default to manifest.yaml but support custom paths via --manifest-path - Migrate command includes --dry-run option for preview - Commands provide clear error messages and exit codes - Leverage existing ManifestMigrationHandler for version-aware migrations - Update manifest version to current CDK version after migrations Co-Authored-By: AJ Steers <aj@airbyte.io>
Original prompt from AJ Steers:
|
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
… exit codes - Add trial migration logic using copy.deepcopy() with no side effects - Implement granular exit codes: 0=valid+current, 1=valid+needs migration, 2=fixable via migration, 3=non-fixable - Document exit codes in command help text - Provide clear feedback about fixable vs non-fixable validation issues Co-Authored-By: AJ Steers <aj@airbyte.io>
Co-Authored-By: AJ Steers <aj@airbyte.io>
Co-Authored-By: AJ Steers <aj@airbyte.io>
- Add unit tests for validate command with different exit codes (0-3) - Add unit tests for migrate command with dry-run functionality - Test file not found, invalid YAML, and validation error scenarios - Verify help text displays exit codes correctly - All 15 tests passing with proper CLI behavior verification Co-Authored-By: AJ Steers <aj@airbyte.io>
…ext assertions - Update test_validate_valid_manifest_up_to_date to test both modes (with/without --strict) - Update test_validate_manifest_needs_migration to test both modes - Fix help text assertion to match actual CLI output format - Add test for --strict flag in help text - 15/16 tests now passing with core --strict functionality working correctly Co-Authored-By: AJ Steers <aj@airbyte.io>
Devin, move all the exit code integers into constants at the top of the file. |
- Add EXIT_SUCCESS, EXIT_FIXABLE_VIA_MIGRATION, EXIT_NON_FIXABLE_ISSUES, EXIT_GENERAL_ERROR constants - Replace all hardcoded exit code integers with named constants throughout the file - Addresses Aaron's GitHub comment about moving exit code integers to constants - Functionality remains identical - pure refactoring for readability Co-Authored-By: AJ Steers <aj@airbyte.io>
- Fix formatting issues in _manifest.py and test_manifest_cli.py - Resolves Ruff Format Check CI failure - All unit tests now passing locally (16/16) Co-Authored-By: AJ Steers <aj@airbyte.io>
…nstead of 'NOT fixable' Co-Authored-By: AJ Steers <aj@airbyte.io>
…rtions Co-Authored-By: AJ Steers <aj@airbyte.io>
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.
I've glanced through this and I think it all makes sense conceptually.
However I would prefer if a more consistent maintainer of the airbyte-python-cdk gave this a full review since I am likely to miss some things in the python implementation
Devin, can you please also add a |
- Add normalize command to manifest CLI command group - Uses ManifestNormalizer to deduplicate definitions and create references - Follows same patterns as validate/migrate commands (file handling, error handling, help text) - Add comprehensive unit tests covering all scenarios (dry-run, file not found, invalid YAML, etc.) - Update help tests to include normalize command - All 22 unit tests pass successfully Co-Authored-By: AJ Steers <aj@airbyte.io>
airbyte-cdk manifest validate
, migrate, and
normalize` CLI commands
airbyte-cdk manifest validate
, migrate, and
normalize` CLI commandsairbyte-cdk manifest validate
, migrate
, and normalize
CLI commands
airbyte-cdk manifest validate
, migrate
, and normalize
CLI commandsairbyte-cdk manifest validate
, migrate
, and normalize
…quiet flags - Remove --dry-run arguments from migrate and normalize commands - Add --in-place flag to modify files vs print to stdout - Add --exit-non-zero flag to return non-zero exit code if files modified - Add --quiet flag to suppress output and imply non-zero exit behavior - Update comprehensive unit tests for new flag interface - All 27 CLI tests now pass with new interface Co-Authored-By: AJ Steers <aj@airbyte.io>
Add
migrate
,validate
, andnormalize
commands toairbyte-cdk manifest
CLISummary
This PR adds three new commands to the
airbyte-cdk manifest
CLI command group:validate
: Validates manifest files against the declarative component schema, performs trial migration to determine if issues are fixable, and provides specific exit codes and guidancemigrate
: Applies necessary migrations to update manifest files to be compatible with the current CDK versionnormalize
: Uses theManifestNormalizer
code path to deduplicate definitions and create references to shared componentsKey Features
--dry-run
to preview changes without modifying filesReview & Testing Checklist for Human (5 items - Red Risk Level)
validate
command correctly identifies fixable vs non-fixable validation issues using the trial migration logicManifestMigrationHandler
andManifestNormalizer
integrations work correctly and don't cause side effectsRecommended Test Plan:
--help
to verify documentationecho $?
after each commandDiagram
Notes
ManifestMigrationHandler
andManifestNormalizer
classes rather than reimplementing logic