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.

Sign up for GitHub

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

Jump to bottom

Split connector builds. #11148

Merged
merged 17 commits into from
Mar 16, 2022
Merged

Split connector builds. #11148

Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
119dc53
Add filtering for connector build.
davinchia Mar 15, 2022
d2035c1
Remove build condition so we can test the new conditions.
davinchia Mar 15, 2022
8e66294
Add stub db modifications.
davinchia Mar 15, 2022
8ff44a8
Remember to add outputs.
davinchia Mar 15, 2022
8922b97
Remove the test db file. Add a test connector change file.
davinchia Mar 15, 2022
58487bb
Remove test file in top level integrations folder. Add test file in o…
davinchia Mar 15, 2022
6e61884
Remove submodule testing file.
davinchia Mar 15, 2022
d4bc499
The frontend build should not run if its only a connector change. It …
davinchia Mar 15, 2022
a416489
Add proper filter for backend modules. Uncomment build for faster tes…
davinchia Mar 15, 2022
04e3cc5
Is the comment messing this up?
davinchia Mar 15, 2022
6960a04
Test airbyte-integrations correct filter.
davinchia Mar 15, 2022
ae0b1fa
Test airbyte-webappcorrect filter.
davinchia Mar 15, 2022
2d22f45
Test airbyte-webappcorrect filter.
davinchia Mar 15, 2022
11d673d
Uncomment rest of build.
davinchia Mar 15, 2022
4d96903
Remove test files.
davinchia Mar 15, 2022
d83d9ed
Better formatting.
davinchia Mar 15, 2022
300b0ef
Split for better formatting.
davinchia Mar 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 23 additions & 3 deletions .github/workflows/gradle.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)/**'
Copy link
Contributor Author

@davinchia davinchia Mar 15, 2022 •
edited
Loading

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.

Copy link
Contributor

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?

Copy link
Contributor Author

@davinchia davinchia Mar 15, 2022 •
edited
Loading

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.

build:
- '.github/**'
- 'buildSrc/**'
Copy link
Contributor Author

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.

- 'tools/**'
cli:
- 'airbyte-cli/**'
connectors:
- 'airbyte-integrations/**'
db:
- 'airbyte-db/**'
frontend:
- 'airbyte-webapp/**'
- 'airbyte-webapp-e2e-tests/**'
Expand All @@ -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'
Copy link
Contributor Author

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?

|| github.ref == 'refs/heads/master'
timeout-minutes: 10
runs-on: ubuntu-latest
outputs:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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'
Copy link
Contributor Author

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

|| needs.changes.outputs.backend == 'true'
timeout-minutes: 10
runs-on: ubuntu-latest
outputs:
Expand Down