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

connectors-ci: skipped status when metadata upload exits with 5 #28938

Merged

Conversation

alafanechere
Copy link
Contributor

@alafanechere alafanechere commented Aug 1, 2023

What

#28767 changed how we identify steps as skipped.
The exit code that leads to the "skipped" step status is now defined at the Step subclass level and is not global.
It introduced a regression for the MetadataUpload step - when its container exits with 5 it means the upload was skipped because the metadata yaml already exists on the bucket.

How

Declare in MetadataUpload that the skipped exit code is 5

@alafanechere alafanechere requested a review from a team August 1, 2023 19:49
@alafanechere alafanechere enabled auto-merge (squash) August 1, 2023 19:50
Copy link
Contributor

@evantahler evantahler left a comment

Choose a reason for hiding this comment

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

5

@alafanechere alafanechere merged commit 0e0be67 into master Aug 1, 2023
18 checks passed
@alafanechere alafanechere deleted the augustin/connectors-ci/handle-metadata-custom-exit-code branch August 1, 2023 19:55
@evantahler
Copy link
Contributor

I don't really understand why 5...

@alafanechere
Copy link
Contributor Author

alafanechere commented Aug 1, 2023

@evantahler when we implemented the metadata service upload we made its process exit with 5 when the upload was skipped because the file hash is the same on the bucket. 0 when the upload is successful, 1 one it failed.
It just a way for this process run inside a container to tell airbyte-ci that the upload was skipped. We picked 5 because pytest exists with the same status code when all tests are skipped.

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.

None yet

2 participants