Skip to content
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

🐛 airbyte-ci: skip archived connectors pre-emptively when using --modified flag #44786

Merged
merged 5 commits into from
Aug 27, 2024

Conversation

ChristoGrab
Copy link
Contributor

@ChristoGrab ChristoGrab commented Aug 26, 2024

What

Adds logic to pre-emptively skip archived connectors when using the --modified flag in airbyte-ci. This should clean up the test logs in CI and make it easier to reach the test reports:

Screenshot 2024-08-26 at 12 37 49 PM

Resolves #9176

How

  • Create a mapping of active connectors (ie, not archived) to iterate through when checking for modified connectors.
  • Remove conditional logic to verify and log each "archived" connector.

User Impact

This change is not geared toward improving performance; it is intended to reduce clutter in logs when checking PR test reports.

Can this PR be safely reverted and rolled back?

  • YES 💚
  • NO ❌

Copy link

vercel bot commented Aug 26, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
airbyte-docs ⬜️ Ignored (Inspect) Visit Preview Aug 27, 2024 2:02pm

@ChristoGrab ChristoGrab marked this pull request as ready for review August 26, 2024 18:15
@ChristoGrab ChristoGrab requested a review from a team as a code owner August 26, 2024 18:15
Copy link
Contributor

@natikgadzhi natikgadzhi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love this. I think @alafanechere had a PR that did this already? Didn't spot it now. Anyway, LGTM

Comment on lines 27 to 30
active_connectors = {conn for conn in all_connectors if conn.support_level != "archived"}
modified_connectors = set()

for connector in all_connectors:
if connector.support_level == "archived":
main_logger.info(f"Skipping connector '{connector}' due to 'archived' support level.")
continue
for connector in active_connectors:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that the per connector logging message is quite annoying. Would you mind keeping a single log line like f"Skipping {len(all_connectors) - len(active_connectors)} archived connectors". I'd love to keep it explicit somewhere that airbyte-ci filters out archived connectors.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, I added the log as you suggested!

I noticed it was being printed multiple times because I had added the mapping of active connectors to the _find_modified_connectors function, which runs on a loop for each modified file. Fixed it by moving the mapping to the outer get_modified_connectors function, so now we check for and remove archived connectors once before entering the loop.

@ChristoGrab ChristoGrab enabled auto-merge (squash) August 27, 2024 14:11
@ChristoGrab ChristoGrab merged commit e911ab8 into master Aug 27, 2024
35 checks passed
@ChristoGrab ChristoGrab deleted the christo/airbyte-ci-fix-modified branch August 27, 2024 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants