-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
cat / airbyte-ci: improve CAT container orchestration #31699
cat / airbyte-ci: improve CAT container orchestration #31699
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
Current dependencies on/for this PR: This comment was auto-generated by Graphite. |
Before Merging a Connector Pull RequestWow! What a great pull request you have here! 🎉 To merge this PR, ensure the following has been done/considered for each connector added or updated:
If the checklist is complete, but the CI check is failing,
|
e2cb8f8
to
b66416e
Compare
d87ef2e
to
b3c44a4
Compare
source-google-sheets test report (commit
|
Step | Result |
---|---|
Build source-google-sheets docker image for platform(s) linux/amd64 | ✅ |
Unit tests | ✅ |
Acceptance tests | ✅ |
Check our base image is used | ✅ |
Code format checks | ✅ |
Validate metadata for source-google-sheets | ✅ |
Connector version semver check | ✅ |
QA checks | ✅ |
☁️ 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-google-sheets test
7e46301
to
5db9753
Compare
source-postgres test report (commit
|
Step | Result |
---|---|
Build connector tar | ✅ |
Java Connector Unit Tests | ✅ |
Build source-postgres docker image for platform(s) linux/amd64 | ✅ |
Acceptance tests | ✅ |
Java Connector Integration Tests | ✅ |
Validate metadata for source-postgres | ✅ |
Connector version semver check | ✅ |
QA checks | ✅ |
☁️ 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-postgres test
bbb4d89
to
00184ec
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM for the parts that I do understand. It makes sense to me to leverage dagger in this way.
I still don't understand why we are not getting performance boost for python-based sources? Please share, if there are more details around this. LGTM! |
00184ec
to
f0ce56d
Compare
@bazarnov As Python sources make API requests that can be rate-limited I'm afraid that running the CAT tests in parallel can easily burst this rate limit. Feel free to try out on python connectors locally by running |
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
What
Closes #31703
This PR aims at improving how we orchestrate CAT through
airbyte-ci
.We want to:
How
Reducing test cold start duration
connector_container
asession
scoped fixture. It means the tests will reuse the same connector container.tar
allows us to remove the long and costlydocker export
anddagger import
steps.Enabling test concurrency
Pytest has
pytest-xdist
plugin which can distribute tests across multiple CPUs.With the
--numprocesses=auto
flag in thepytest
command this plugin will distribute tests across all the available cores.I don't want to enable this by default on Python connectors as I'm concerned it can have a bad impact on the API connectors' rate limit. So I:
--concurrent-cat
flag onairbyte-ci connectors test
command. Defaulting to false.Performance boost on
source-postgres
(with concurrency).This change makes
source-postgres
CAT execution run in 01mn56s. Which is a ~2mn boost compared to the current 03mn54 execution onmaster
.Perfomance boost on python connectors (no concurrency)
I used
source-google-sheets
for testing.As I said, test concurrency is disabled by default for Python connectors.
We still get a ~1mn CAT boost due to a lower test cold start.
On master: 3mn11s
On this branch: 2mn07
Recommended reading order
airbyte-integrations/bases/connector-acceptance-test/*
airbyte-ci
:airbyte-ci/connectors/pipelines/*
🚨 User Impact 🚨