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: fix pip install error #29156

Merged
merged 5 commits into from
Aug 7, 2023

Conversation

alafanechere
Copy link
Contributor

@alafanechere alafanechere commented Aug 7, 2023

What

Closes #29147

Fixes two problems:

  • with_exec("pip", "install", ...) was called too early: it was evaluated during the get_file_contents function, this function raised an unrelated errors because it first executed the lazy with_exec(["pip", "install"]) before processing the file content
  • When a connector does not have a setup.py or requirements.txt using get_file_contents raises a dagger exception that we correctly handled but Dagger will still consider this as a failure on their UI when the file does not exist.

How

  • Check if the file setup.py or requirements.txt exist with the check_path_in_workdir function. This function does not raise a Dagger exception if the file does not exist.
  • Check if the file setup.py or requirements.txt exist before appending with_exec(["pip", "install", ...]) to the container: it solved the early raise of pip install error that led to sentry error capture

@octavia-squidington-iii
Copy link
Collaborator

source-intercom test report (commit 369002556b) - ❌

⏲️ Total pipeline duration: 01mn08s

Step Result
Validate airbyte-integrations/connectors/source-intercom/metadata.yaml
Connector version semver check
QA checks
Code format checks
Connector package install

🔗 View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-intercom test

@octavia-squidington-iii
Copy link
Collaborator

source-openweather test report (commit 369002556b) - ✅

⏲️ Total pipeline duration: 02mn18s

Step Result
Validate airbyte-integrations/connectors/source-openweather/metadata.yaml
Connector version semver check
QA checks
Code format checks
Connector package install
Build source-openweather docker image for platform linux/x86_64
Unit tests
Acceptance tests

🔗 View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-openweather test

@alafanechere
Copy link
Contributor Author

Manual testing

I triggered a test pipeline for source-intercom which is failing the pip install operation for unrelated reasons.
No new sentry event was sent for this error on this run: the pip install failure is correctly handled.

This reverts commit 3690025.
@octavia-squidington-iii octavia-squidington-iii removed the area/connectors Connector related issues label Aug 7, 2023
@alafanechere alafanechere requested a review from a team August 7, 2023 16:56
@alafanechere alafanechere enabled auto-merge (squash) August 7, 2023 16:58
@alafanechere alafanechere merged commit 1de0add into master Aug 7, 2023
18 checks passed
@alafanechere alafanechere deleted the augustin/connectors-ci/fix-pip-install-errors branch August 7, 2023 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ExecError: process "python -m pip install ." did not complete successfully: exit code: 1
3 participants