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: don't require extra permissions or --is-ci flag to upload reports to GCS bucket #48824

Merged

Conversation

aaronsteers
Copy link
Collaborator

@aaronsteers aaronsteers commented Dec 5, 2024

What

In order to enable report uploads without granting a large number of new permissions, this change decouples the report upload feature from the --is-ci flag. Originally, I tried to just enable --is-ci but then a number of other environment variables became requires, such as dockerhub username and password, which are not needed in our use cases.

The fix here is simply to upload reports as long as --ci-report-upload-bucket and the GCP creds are both set.

This is tested successfully in Python CDK and ready to go!

Tested here:

How

Review guide

User Impact

Can this PR be safely reverted and rolled back?

  • YES 💚
  • NO ❌

Copy link

vercel bot commented Dec 5, 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 Dec 5, 2024 9:10pm

@aaronsteers aaronsteers changed the title don't require "is_ci" flag to upload reports to gcs bucket airbyte-ci: don't require extra permissions or --is-ci flag to upload reports to GCS bucket Dec 5, 2024
@aaronsteers aaronsteers marked this pull request as ready for review December 5, 2024 20:25
@aaronsteers aaronsteers requested a review from a team as a code owner December 5, 2024 20:25
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.

Seems fine — the only env where you would have gcp creds is probably CI either way.

Bump the versions of airbyte-ci please.

@aaronsteers aaronsteers enabled auto-merge (squash) December 5, 2024 21:10
@aaronsteers aaronsteers merged commit 7a45e94 into master Dec 5, 2024
32 checks passed
@aaronsteers aaronsteers deleted the aj/airbyte-ci/allow-report-uploads-with-fewer-permissions branch December 5, 2024 21:33
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.

2 participants