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: do not eagerly fetch connector secrets #38615

Conversation

alafanechere
Copy link
Contributor

@alafanechere alafanechere commented May 23, 2024

What

#38322 introduces an edge case bug when airbyte-ci connectors test is run with --skip-step or --only-step:
Even if some test suites are skipped the existing logic tried to fetch the secrets from the secret stores.
It breaks community CI and other workflow which only run a portion of the full test pipeline.

Solution

Fetch secerts according to the run_step_options.
A lot of secret related logic moved to the ConnectorContext class as it's where these options are exposed.

@alafanechere alafanechere requested a review from a team as a code owner May 23, 2024 14:21
Copy link

vercel bot commented May 23, 2024

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

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
airbyte-docs ⬜️ Ignored (Inspect) Visit Preview May 23, 2024 4:18pm

Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @alafanechere and the rest of your teammates on Graphite Graphite

@alafanechere alafanechere force-pushed the augustin/05-23-airbyte-ci_do_not_eagerly_fetch_connector_secrets branch from 8ec5f67 to 8133703 Compare May 23, 2024 14:22
Copy link
Contributor

@clnoll clnoll left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@bnchrch bnchrch left a comment

Choose a reason for hiding this comment

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

Kudos to the quick resolution

Im not the biggest fan of moving this to the context store (simply because if we keep this pattern up then context.py becomes a very very large file)

But! Lets get a fix out.

Approved

@alafanechere alafanechere enabled auto-merge (squash) May 23, 2024 14:31
@alafanechere alafanechere force-pushed the augustin/05-23-airbyte-ci_do_not_eagerly_fetch_connector_secrets branch from 8133703 to 22d6e0b Compare May 23, 2024 16:18
@alafanechere alafanechere merged commit 311dcce into master May 23, 2024
29 checks passed
@alafanechere alafanechere deleted the augustin/05-23-airbyte-ci_do_not_eagerly_fetch_connector_secrets branch May 23, 2024 16:38
Copy link

sentry-io bot commented May 23, 2024

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ SecretNotFoundError: Secret SECRET_DESTINATION-SNOWFLAKE_COPY_AZURE_BLOB__CREDS can't be retrieved as airbyte-connecto... pipelines.airbyte_ci.connectors.context in _han... View Issue

Did you find this useful? React with a 👍 or 👎

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