Skip to content

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

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

Conversation

devin-ai-integration[bot]
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot commented Jun 27, 2025

Add migrate, validate, and normalize commands to airbyte-cdk manifest CLI

Summary

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 guidance
  • migrate: Applies necessary migrations to update manifest files to be compatible with the current CDK version
  • normalize: Uses the ManifestNormalizer code path to deduplicate definitions and create references to shared components

Key Features

  • Version-aware migration filtering: Only applies migrations newer than the manifest's declared version
  • Trial migration in validate: Determines if validation issues are fixable via migration (exit code 1) vs unfixable (exit code 2)
  • Granular exit codes: Different codes for success (0), fixable issues (1), unfixable validation errors (2), and general errors (3)
  • Dry-run support: All commands support --dry-run to preview changes without modifying files
  • Comprehensive error handling: Handles missing files, invalid YAML, non-dictionary YAML, and validation errors
  • Help text with exit codes: Documents all exit codes and their meanings in command help

Review & Testing Checklist for Human (5 items - Red Risk Level)

  • Manual end-to-end testing: Test all three commands with real manifest files of varying complexity and states (valid, needs migration, has validation errors)
  • Exit code verification: Verify that each command returns the correct exit codes as documented in the help text for different scenarios
  • Trial migration accuracy: Test that validate command correctly identifies fixable vs non-fixable validation issues using the trial migration logic
  • Error handling robustness: Test edge cases like missing files, malformed YAML, non-dictionary YAML, and permission errors
  • Integration verification: Confirm that ManifestMigrationHandler and ManifestNormalizer integrations work correctly and don't cause side effects

Recommended Test Plan:

  1. Create test manifest files in different states (valid, needs migration, invalid)
  2. Run each command with --help to verify documentation
  3. Test both dry-run and actual execution modes
  4. Verify exit codes match expectations using echo $? after each command
  5. Test with edge cases (empty files, malformed YAML, missing files)

Diagram

graph TB
    CLI[airbyte-cdk CLI] --> MG[manifest command group]
    MG --> V[validate command]:::major-edit
    MG --> M[migrate command]:::major-edit  
    MG --> N[normalize command]:::major-edit
    
    V --> VL[Validation Logic]
    V --> TM[Trial Migration]:::major-edit
    V --> EC1[Exit Codes 0,1,2,3]:::major-edit
    
    M --> MH[ManifestMigrationHandler]:::context
    M --> VU[Version Update]:::context
    
    N --> MN[ManifestNormalizer]:::context
    N --> DD[Deduplicate Definitions]:::context
    
    VL --> Schema[Declarative Component Schema]:::context
    TM --> MH
    
    Tests[unit_tests/cli/test_manifest_cli.py]:::major-edit
    Tests --> V
    Tests --> M  
    Tests --> N
    
    Impl[airbyte_cdk/cli/airbyte_cdk/_manifest.py]:::major-edit
    Impl --> V
    Impl --> M
    Impl --> N
    
    subgraph Legend
        L1[Major Edit]:::major-edit
        L2[Minor Edit]:::minor-edit
        L3[Context/No Edit]:::context
    end
    
    classDef major-edit fill:#90EE90
    classDef minor-edit fill:#87CEEB  
    classDef context fill:#FFFFFF
Loading

Notes

  • Requirements Evolution: This PR evolved from the initial validate/migrate requirements to include the normalize command, ensuring all three commands follow consistent patterns
  • Exit Code Strategy: Uses constants defined at module level for maintainability and consistency across commands
  • Error Handling: Follows existing CLI patterns with rich formatting and user-friendly error messages
  • Testing Coverage: Added 22 unit tests covering all commands, error scenarios, and help text validation
  • Integration Approach: Leverages existing ManifestMigrationHandler and ManifestNormalizer classes rather than reimplementing logic

- 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>
Copy link
Contributor Author

Original prompt from AJ Steers:

Received message in Slack channel #ask-devin-ai:

@Devin - Can you update the python CDK CLI, specifically the `airbyte-cdk manifest` command group to add a 'migrate' and 'validate' command.
• The 'validate' command should fail if the manifest is not valid.
• The 'migrate' command should apply any necessarily migrations to the `manifest.yaml` so that it is compatible with the latest version.
• The 'validate' command should fail and suggest migration if needed.

Copy link
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@github-actions github-actions bot added the enhancement New feature or request label Jun 27, 2025
Copy link

github-actions bot commented Jun 27, 2025

PyTest Results (Fast)

3 703 tests  +27   3 692 ✅ +27   6m 30s ⏱️ +7s
    1 suites ± 0      11 💤 ± 0 
    1 files   ± 0       0 ❌ ± 0 

Results for commit f3c8ef2. ± Comparison against base commit 0b4195b.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Jun 27, 2025

PyTest Results (Full)

3 706 tests  +27   3 695 ✅ +27   18m 4s ⏱️ -1s
    1 suites ± 0      11 💤 ± 0 
    1 files   ± 0       0 ❌ ± 0 

Results for commit f3c8ef2. ± Comparison against base commit 0b4195b.

♻️ This comment has been updated with latest results.

devin-ai-integration bot and others added 4 commits June 27, 2025 03:45
… 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>
- 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>
@aaronsteers
Copy link
Contributor

Devin, move all the exit code integers into constants at the top of the file.

devin-ai-integration bot and others added 4 commits June 27, 2025 05:28
- 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>
Copy link
Contributor

@lmossman lmossman left a 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

@aaronsteers
Copy link
Contributor

Devin, can you please also add a normalize command, similar to upgrade but using the manifest_normalizer.py code path?

- 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>
@aaronsteers aaronsteers changed the title feat(cli): Add manifest validate and migrate commands feat(cli): Add airbyte-cdk manifest validate, migrate, and normalize` CLI commands Jun 27, 2025
@aaronsteers aaronsteers changed the title feat(cli): Add airbyte-cdk manifest validate, migrate, and normalize` CLI commands feat(cli): Add airbyte-cdk manifest validate, migrate, and normalize CLI commands Jun 27, 2025
@aaronsteers aaronsteers changed the title feat(cli): Add airbyte-cdk manifest validate, migrate, and normalize CLI commands feat(cli): Add CLI commands: airbyte-cdk manifest validate, migrate, and normalize Jun 27, 2025
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants