-
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
Changes from all commits
119dc53
d2035c1
8e66294
8ff44a8
8922b97
58487bb
6e61884
d4bc499
a416489
04e3cc5
6960a04
ae0b1fa
2d22f45
11d673d
4d96903
d83d9ed
300b0ef
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,20 +27,29 @@ jobs: | |
backend: ${{ steps.filter.outputs.backend }} | ||
build: ${{ steps.filter.outputs.build }} | ||
cli: ${{ steps.filter.outputs.cli }} | ||
connectors: ${{ steps.filter.outputs.connectors }} | ||
db: ${{ steps.filter.outputs.db }} | ||
frontend: ${{ steps.filter.outputs.frontend }} | ||
steps: | ||
- name: Checkout Airbyte | ||
uses: actions/checkout@v2 | ||
- uses: dorny/paths-filter@v2 | ||
id: filter | ||
with: | ||
# Note, the following glob expression within a filters are ORs. | ||
filters: | | ||
backend: | ||
- 'airbyte-**/**' | ||
- 'airbyte-!(integrations|webapp|webapp-e2e-tests)/**' | ||
build: | ||
- '.github/**' | ||
- 'buildSrc/**' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I realised these are part of the 'build' too. |
||
- 'tools/**' | ||
cli: | ||
- 'airbyte-cli/**' | ||
connectors: | ||
- 'airbyte-integrations/**' | ||
db: | ||
- 'airbyte-db/**' | ||
frontend: | ||
- 'airbyte-webapp/**' | ||
- 'airbyte-webapp-e2e-tests/**' | ||
|
@@ -55,10 +64,17 @@ jobs: | |
# - run: | | ||
# echo '${{ toJSON(needs) }}' | ||
|
||
## BUILDS | ||
|
||
# Gradle Build (Connectors Base) | ||
# In case of self-hosted EC2 errors, remove this block. | ||
|
||
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 commentThe 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? |
||
|| github.ref == 'refs/heads/master' | ||
timeout-minutes: 10 | ||
runs-on: ubuntu-latest | ||
outputs: | ||
|
@@ -157,7 +173,9 @@ jobs: | |
- start-connectors-base-build-runner # required to get output from the start-runner job | ||
- build-connectors-base # required to wait when the main job is done | ||
runs-on: ubuntu-latest | ||
if: ${{ always() }} # required to stop the runner even if the error happened in the previous jobs | ||
# Always is required to stop the runner even if the previous job has errors. However always() runs even if the previous step is skipped. | ||
# Thus, we check for skipped here. | ||
if: ${{ always() && needs.start-connectors-base-build-runner.result != 'skipped'}} | ||
steps: | ||
- name: Configure AWS credentials | ||
uses: aws-actions/configure-aws-credentials@v1 | ||
|
@@ -504,7 +522,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 commentThe reason will be displayed to describe this comment to others. Learn more. splitting this for better formatting |
||
|| needs.changes.outputs.backend == 'true' | ||
timeout-minutes: 10 | ||
runs-on: ubuntu-latest | ||
outputs: | ||
|
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.