-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Isolate our build so that the feedback loop is faster #23411
Conversation
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.
It seems like a partial win since we don't often do changes in isolation and this will probably trigger the build for "Connectors Base: Build" when we update some sources. We will need to find a way to avoid running "Connectors Base: Build" when modifying a source that is relying on the airbyte-cdk
- uses: actions/setup-python@v4 | ||
with: | ||
python-version: "3.9" | ||
|
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.
"Connectors Base: Build" was adding "Install Pyenv" (introduced here) but it's not so clear to me why we would need this
include ':airbyte-db:jooq' // transitively used by airbyte-workers. | ||
include ':airbyte-persistence:job-persistence' // transitively used by airbyte-workers. | ||
if (!System.getenv().containsKey("SUB_BUILD") || System.getenv().get("SUB_BUILD") == "CDK" || System.getenv().get("SUB_BUILD") == "ALL_CONNECTORS") { | ||
include ':airbyte-commons' // this wouldn't be necessary if it wasn't from https://github.com/airbytehq/airbyte/blob/645558b74aab0b91fda1b4628b37b7095d92b4cc/build.gradle |
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.
This seems very odd but I didn't want to investigate this since the build is still somewhat fast compared to before. Note that this seems like another improvement we could do though
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.
Nice! Small surgical change. Two small requests;
- Could you also confirm you looked at the docs anywhere it says
SUB_BUILD=CONNECTORS_BASE
to make sure that doc is still current? - Also, the PR description should mention the gains in build speed 😎
Also added similar logic to #23415 |
* Isolate our build so that the feedback loop is faster * Improve gradle build condition * Update documentation * Adding auto-commit of formatting changes (as airbytehq#23415)
What
We want the extensibility team to have their own checks and be impacted as less as possible with the builds of other.
The build is now 4.5 times faster! This increases the feedback loop and reduce the risk of occurrence of https://github.com/airbytehq/airbyte-internal-issues/issues/1345
How
By creating our own check and moving airbyte-cdk changes under this check, we will:
Recommended reading order
settings.gradle
: Gradle build changes.github/workflows/gradle.yml
: GitHub checks changes