-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
Split connector builds. #11148
Split connector builds. #11148
Conversation
Example of the connector build not running since there are no changes: https://github.com/airbytehq/airbyte/runs/5549783444?check_suite_focus=true I removed the build condition to test this. |
Example of the connector build running because of changes in the db module: https://github.com/airbytehq/airbyte/runs/5549837857?check_suite_focus=true |
Example of connector build running because of changes in the integrations module: https://github.com/airbytehq/airbyte/runs/5549882923?check_suite_focus=true |
…ne of the submodules.
Example of connector build running because of change in connector submodule: https://github.com/airbytehq/airbyte/runs/5549907212?check_suite_focus=true |
@@ -39,8 +41,14 @@ jobs: | |||
- 'airbyte-**/**' | |||
build: | |||
- '.github/**' | |||
- 'buildSrc/**' |
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.
I realised these are part of the 'build' too.
…should run if it is a backend change alongside the connector change.
filters: | | ||
backend: | ||
- 'airbyte-**/**' | ||
- 'airbyte-!(integrations|webapp|webapp-e2e-tests)/**' |
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.
@cgardens I've improved the backend filter so it ignores the non-backend modules. This should further reduce excessive FE build runs. Lmk if I missed something out.
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 feels like the right way to do this would be to use actual build caching in gradle instead of setting these manual filters. Not for this PR but is that the right North Star?
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.
Yes. We do use gradle's caching under the hood. These filters happen at the build-infrastructure level instead of the build level. It helps us avoid running the same gradle tasks X many times on the various build pipelines.
We can try to push this into gradle (though honestly not much more we can do). However that still does not give teams build setup isolation from each other.
@@ -504,7 +520,9 @@ jobs: | |||
start-frontend-test-runner: | |||
name: Start Frontend Test EC2 Runner | |||
needs: changes | |||
if: needs.changes.outputs.frontend == 'true' || needs.changes.outputs.build == 'true' || needs.changes.outputs.backend == 'true' || github.ref == 'refs/heads/master' | |||
if: | | |||
needs.changes.outputs.frontend == 'true' || needs.changes.outputs.build == 'true' || github.ref == 'refs/heads/master' |
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.
splitting this for better formatting
start-connectors-base-build-runner: | ||
name: "Connectors Base: Start Build EC2 Runner" | ||
needs: changes | ||
if: | | ||
needs.changes.outputs.build == 'true' || needs.changes.outputs.connectors == 'true' || needs.changes.outputs.db == 'true' |
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.
my understanding is the top level db module is the only module we want to rest the connector build on if it's changes. is this correct?
PTAL @sherifnada @cgardens @jrhizor @supertopher following the previous split mechanism for the connector builds. I think the DB module is the top level module we should trigger the connector build on - lmk if I missed something out. FYI @timroes since this does touch the frontend build too. This PT prevents connector build triggering on platform changes but does not prevent platform build triggering on connector changes. That will be the last follow up. I'm doing this piecemeal to minimise the changeset. |
What
Follow up to #11064 for the connector builds. After this change, connector builds will only trigger if
How
Recommended reading order
🚨 User Impact 🚨
Are there any breaking changes? What is the end result perceived by the user? If yes, please merge this PR with the 🚨🚨 emoji so changelog authors can further highlight this if needed.
Pre-merge Checklist
Expand the relevant checklist and delete the others.
New Connector
Community member or Airbyter
airbyte_secret
./gradlew :airbyte-integrations:connectors:<name>:integrationTest
.README.md
bootstrap.md
. See description and examplesdocs/SUMMARY.md
docs/integrations/<source or destination>/<name>.md
including changelog. See changelog exampledocs/integrations/README.md
airbyte-integrations/builds.md
Airbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.
/test connector=connectors/<name>
command is passing/publish
command described hereUpdating a connector
Community member or Airbyter
airbyte_secret
./gradlew :airbyte-integrations:connectors:<name>:integrationTest
.README.md
bootstrap.md
. See description and examplesdocs/integrations/<source or destination>/<name>.md
including changelog. See changelog exampleAirbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.
/test connector=connectors/<name>
command is passing/publish
command described hereConnector Generator
-scaffold
in their name) have been updated with the latest scaffold by running./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplates
then checking in your changesTests
Unit
Put your unit tests output here.
Integration
Put your integration tests output here.
Acceptance
Put your acceptance tests output here.