feat(ci): replace hard-to-maintain `get-modified-connectors.sh` shell script with Python implementation having built-in tests #67560

aaronsteers · 2025-10-08T00:11:35Z

What

Replaces the bash script get-modified-connectors.sh with a new Python implementation get_connector_matrix.py to improve maintainability and add certification filtering capabilities. This script has had a number of issues over the past few months, which take a toll on engineering capacity. This update rewrites the script in poetry, using uv to run the script, and adding doctest tests, which allow each function in the script to have their own inline examples-as-tests.

Note:

The new script does not require pre-installing python. The uv tool and the uv-based shebang will bootstrap a compatible python version if none is available. This means callers can directly run the script with no need to pre-install anything besides uv itself (brew install uv). I wrote more about this here: The Modern Ways to (not) Install Python and Virtual Environments - DEV Community
Tests are inline to each function's docstring using doctest. Simply run ./poe-tasks/get_connector_matrix.py --run-tests to test all functions.

Requested by: @aaronsteers
Devin session: https://app.devin.ai/sessions/da08211cf74a4ac5980f874209e7f0ce

How

New Python script: get_connector_matrix.py uses PEP 723 inline metadata with uv for portable execution on Python 3.12
Enhanced functionality: Adds --certified and --no-certified flags to filter connectors by support level
Efficient caching: Uses @lru_cache decorated get_manifest_dict() function to cache metadata.yaml reads
Comprehensive testing: Includes 19 doctests covering all utility functions
Updated task configuration: Modified poethepoet task to use new script with POSIX-compatible shell wrapper

Review guide

poe-tasks/get_connector_matrix.py - New Python implementation
- Git detection logic (lines 55-111) - verify it matches original bash behavior
- Metadata.yaml parsing functions (lines 216-303) - check YAML structure assumptions
- Filtering logic (lines 306-353) - verify Java and certification detection accuracy
- CLI argument handling (lines 459-491) - ensure all original args are preserved
poe-tasks/repo-root-tasks.toml - Updated task configuration
- Shell wrapper logic (lines 31-40) - verify boolean flag conversion works correctly
- Argument definitions (lines 42-50) - check all original functionality is preserved

Critical areas to verify:

Output format compatibility with existing GitHub Actions workflows
Git command behavior across different repository states
YAML parsing robustness for different metadata.yaml structures
Shell wrapper handling of poethepoet boolean flags

User Impact

Positive:

Same command interface as before (poe get-modified-connectors)
New certification filtering capabilities (--certified, --no-certified)
More maintainable and testable codebase with comprehensive doctests
Better error handling and user feedback

Potential negative:

New dependency on uv and Python 3.12 (though these should be available in CI)
Added PyYAML dependency for metadata parsing

Can this PR be safely reverted and rolled back?

YES 💚

The original bash script still exists and the poethepoet task can be easily reverted to use it. No breaking changes to the external API.

⚠️ Important: This PR replaces a critical CI component. The new implementation has been tested with doctests and manual verification, but thorough testing in the actual CI environment is recommended before full rollout.

…tion - Replace get-modified-connectors.sh with get_connector_matrix.py - Add support for filtering connectors by certification status (--certified/--no-certified) - Use PEP 723 inline metadata with uv for portable execution - Pin to Python 3.12 for stability - Include doctests for all utility functions (19 tests total) - Add @lru_cache decorated get_manifest_dict() to efficiently cache metadata.yaml reads - Refactor is_java_connector() and is_certified_connector() to use cached manifest reader - Update poethepoet tasks to use new Python script with POSIX-compatible shell wrapper - Add Contributing section to docstring with doctest guidelines The new script maintains full backward compatibility with the original bash script while adding certification filtering capabilities and improved testability. Co-Authored-By: AJ Steers <[email protected]>

devin-ai-integration · 2025-10-08T00:11:40Z

Original prompt from AJ Steers

Received message in Slack channel #dev-ci:

@Devin - in the connector CI workflow for the main airbyte repo, consolidate the JVM and non-jvm test workflows into a single workflow. Use the "get-language" Poe command to detect and selectively skip tests that are only relevant for one type or the other.
Thread URL: https://airbytehq-team.slack.com/archives/C08PWJ16LUC/p1759865967332799?thread_ts=1759865967.332799

devin-ai-integration · 2025-10-08T00:11:41Z

🤖 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 · 2025-10-08T00:12:02Z

👋 Greetings, Airbyte Team Member!

Here are some helpful tips and reminders for your convenience.

Helpful Resources

PR Slash Commands

Airbyte Maintainers (that's you!) can execute the following slash commands on your PR:

/format-fix - Fixes most formatting issues.
/update-connector-cdk-version connector=<CONNECTOR_NAME> - Updates the specified connector to the latest CDK version.
Example: /update-connector-cdk-version connector=destination-bigquery
/bump-version - Bumps connector versions.
- You can specify a custom changelog by passing changelog. Example: /bump-version changelog="My cool update"
- Leaving the changelog arg blank will auto-populate the changelog from the PR title.
/run-cat-tests - Runs legacy CAT tests (Connector Acceptance Tests)
/build-connector-images - Builds and publishes a pre-release docker image for the modified connector(s).
/poe connector source-example lock - Run the Poe lock task on the source-example connector, committing the results back to the branch.
/poe source example lock - Alias for /poe connector source-example lock.
/poe source example use-cdk-branch my/branch - Pin the source-example CDK reference to the branch name specified.
/poe source example use-cdk-latest - Update the source-example CDK dependency to the latest available version.

📝 Edit this welcome message.

Replace argparse with Typer for more concise and modern CLI definition. - Add typer to PEP 723 inline dependencies - Refactor main() function to use @app.command() decorator - Use typer.Option() for all CLI arguments - Maintain all existing functionality (all 19 doctests pass) - All verification commands work correctly with Typer CLI Co-Authored-By: AJ Steers <[email protected]>

Replace complex bash wrapper with simple cmd passthrough. The get-modified-connectors poe task no longer needs the bash wrapper that conditionally builds command arguments since Typer now handles all CLI parsing. This reduces the task definition from 25 lines to 2 lines. All verification commands pass: - poe get-modified-connectors --json - poe get-modified-connectors --certified --json - poe get-modified-connectors --java --json - poe get-modified-connectors --files-list ... --json Co-Authored-By: AJ Steers <[email protected]>

…trix

…s://github.com/airbytehq/airbyte into devin/1759881721-replace-bash-connector-matrix

Copilot

Pull Request Overview

This PR replaces the bash script get-modified-connectors.sh with a new Python implementation get_connector_matrix.py to improve maintainability and add certification filtering capabilities.

Implements Python script with PEP 723 inline metadata for portable execution
Adds --certified and --no-certified flags for filtering connectors by support level
Includes comprehensive doctests for all utility functions and LRU caching for metadata parsing

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
poe-tasks/get_connector_matrix.py	New Python implementation with git detection, YAML parsing, filtering logic, and comprehensive doctests
poe-tasks/repo-root-tasks.toml	Updated task configuration to use new Python script instead of bash script

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

poe-tasks/get_connector_matrix.py

poe-tasks/repo-root-tasks.toml

poe-tasks/get_connector_matrix.py

- Fix undefined local_cdk variable bug in get_modified_connectors() - Update all workflow files to use get_connector_matrix.py - Update CLI arg from --json to --json-matrix - Replace references in: - connector-ci-checks.yml (4 references) - build-connector-images-command.yml (1 reference) - cdk-connector-compatibility-test.yml (1 reference) - publish_connectors.yml (1 reference) All tests pass and no references to get-modified-connectors.sh remain. Co-Authored-By: AJ Steers <[email protected]>

Co-Authored-By: AJ Steers <[email protected]>

edgao

looks right,

some logistical questions

as a python noob, how does one run the docstring tests 😅
and do we want those to be triggered on CI? or is that overkill, and we can just leave it up to people to remember to do it if they edit get_connector_matrix.py

edgao · 2025-10-09T22:51:52Z

poe-tasks/get_connector_matrix.py

+        return {}
+
+
+def find_local_cdk_connectors() -> list[str]:


not a blocker for this PR, but we recently changed how this gets declared >.> so reminder to me to update this method later, I guess

Co-Authored-By: AJ Steers <[email protected]>

devin-ai-integration · 2025-10-09T22:59:39Z

Thanks for the review! To answer your questions:

How to run the docstring tests: The module docstring already has instructions in the "Contributing" section:
```
poe get-modified-connectors --run-tests
```
Or directly:
```
uv run poe-tasks/get_connector_matrix.py --run-tests
```
Should doctests run on CI? I'll defer to you and the team on this decision. The tests are quick to run (12 tests in <1 second), so adding them to CI wouldn't add much overhead. On the other hand, if the file is rarely edited, manual testing might be sufficient.

Note: I just pushed a commit to fix the ruff-format issue that was caught by CI on the "tidy code" commit.

devin-ai-integration bot assigned aaronsteers Oct 8, 2025

devin-ai-integration bot and others added 8 commits October 8, 2025 00:21

tidy usage

798e1d6

Merge branch 'master' into devin/1759881721-replace-bash-connector-ma…

c2019c0

…trix

Merge branch 'devin/1759881721-replace-bash-connector-matrix' of http…

5570061

…s://github.com/airbytehq/airbyte into devin/1759881721-replace-bash-connector-matrix

mucho refactor, mucho tidying up

cacd269

moar tests

98c4bf7

clean up

27b1969

aaronsteers requested review from Copilot, davinchia and dbgold17 October 8, 2025 05:15

Copilot AI reviewed Oct 8, 2025

View reviewed changes

poe-tasks/get_connector_matrix.py Outdated Show resolved Hide resolved

poe-tasks/repo-root-tasks.toml Show resolved Hide resolved

poe-tasks/get_connector_matrix.py Outdated Show resolved Hide resolved

aaronsteers changed the title ~~feat(ci): replace bash connector matrix script with Python implementation~~ feat(ci): replace untestable get-modified-connectors shell script with Python implementation using built-in tests Oct 8, 2025

aaronsteers changed the title ~~feat(ci): replace untestable get-modified-connectors shell script with Python implementation using built-in tests~~ feat(ci): replace untestable get-modified-connectors shell script with Python implementation having built-in tests Oct 8, 2025

chore(ci): remove unused click import from connector matrix script

1e40f40

Co-Authored-By: AJ Steers <[email protected]>

aaronsteers changed the title ~~feat(ci): replace untestable get-modified-connectors shell script with Python implementation having built-in tests~~ feat(ci): replace hard-to-maintain get-modified-connectors shell script with Python implementation having built-in tests Oct 8, 2025

aaronsteers requested a review from edgao October 8, 2025 05:33

aaronsteers changed the title ~~feat(ci): replace hard-to-maintain get-modified-connectors shell script with Python implementation having built-in tests~~ feat(ci): replace hard-to-maintain get-modified-connectors.sh shell script with Python implementation having built-in tests Oct 8, 2025

devin-ai-integration bot and others added 2 commits October 8, 2025 05:41

chore(ci): remove deprecated get-modified-connectors.sh bash script

9b280ab

Co-Authored-By: AJ Steers <[email protected]>

tidy code

864d762

edgao approved these changes Oct 9, 2025

View reviewed changes

fix(ci): apply ruff-format to get_connector_matrix.py

611a637

Co-Authored-By: AJ Steers <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(ci): replace hard-to-maintain `get-modified-connectors.sh` shell script with Python implementation having built-in tests #67560

feat(ci): replace hard-to-maintain `get-modified-connectors.sh` shell script with Python implementation having built-in tests #67560

Uh oh!

aaronsteers commented Oct 8, 2025 •

edited

Loading

Uh oh!

devin-ai-integration bot commented Oct 8, 2025

Uh oh!

devin-ai-integration bot commented Oct 8, 2025

Uh oh!

github-actions bot commented Oct 8, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

edgao left a comment

Uh oh!

edgao Oct 9, 2025

Uh oh!

devin-ai-integration bot commented Oct 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

feat(ci): replace hard-to-maintain get-modified-connectors.sh shell script with Python implementation having built-in tests #67560

Are you sure you want to change the base?

feat(ci): replace hard-to-maintain get-modified-connectors.sh shell script with Python implementation having built-in tests #67560

Uh oh!

Conversation

aaronsteers commented Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

How

Review guide

User Impact

Can this PR be safely reverted and rolled back?

Uh oh!

devin-ai-integration bot commented Oct 8, 2025

Uh oh!

devin-ai-integration bot commented Oct 8, 2025

🤖 Devin AI Engineer

Uh oh!

github-actions bot commented Oct 8, 2025

👋 Greetings, Airbyte Team Member!

Helpful Resources

PR Slash Commands

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

edgao left a comment

Choose a reason for hiding this comment

Uh oh!

edgao Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration bot commented Oct 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

feat(ci): replace hard-to-maintain `get-modified-connectors.sh` shell script with Python implementation having built-in tests #67560

feat(ci): replace hard-to-maintain `get-modified-connectors.sh` shell script with Python implementation having built-in tests #67560

aaronsteers commented Oct 8, 2025 •

edited

Loading