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.

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

feat(build): conditional ci #9673

Merged

Conversation

david-leifker
Copy link
Collaborator

@david-leifker david-leifker commented Jan 19, 2024

This PR introduces an action which determines which major component has changes (frontend, backend, ingestion, setup jobs). Based on the changes it will selectively run CI jobs.

In the unified workflow:

  • images are pulled from head/head-slim and swapped with the expected image tags when needed just before smoke-tests are run
  • the smoke test matrix is also dynamic based on the changed component
  • when publishing is enabled everything is run to ensure images are built and tested

A new custom action local to this repository will maintain the mapping from file paths to major component. That action will then be used in any workflow which needs selective jobs/steps. Currently this means the primary unified image build and the build and test workflows.

For example: a frontend change would trigger the build of the frontend container and only run cypress tests.

Scenarios

No change to any major component

Screenshot 2024-01-19 at 4 00 08 PM

Frontend change only

  • Build frontend image
  • Runs cypress tests

Screenshot 2024-01-19 at 4 37 51 PM

Ingestion change only

  • Build ingestion image
  • Runs non-cypress tests

Screenshot 2024-01-19 at 5 04 34 PM

Backend change

  • Build backend images (exclude frontend and ingestion images)
  • Runs all smoke tests

Screenshot 2024-01-19 at 5 19 44 PM

Checklist

  • The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
  • Links to related issues (if applicable)
  • Tests for the changes have been added/updated (if applicable)
  • Docs related to the changes have been added/updated (if applicable). If a new feature has been added a Usage Guide has been added for the same.
  • For any breaking change/potential downtime/deprecation/big changes an entry has been made in Updating DataHub

@github-actions github-actions bot added the devops PR or Issue related to DataHub backend & deployment label Jan 19, 2024
@david-leifker david-leifker force-pushed the build-ci-conditionals branch 2 times, most recently from 738cdc0 to ba61736 Compare January 19, 2024 16:44
@david-leifker david-leifker marked this pull request as ready for review January 19, 2024 16:49
@david-leifker david-leifker marked this pull request as draft January 19, 2024 16:49
- "datahub-web-react/**"
- "smoke-test/tests/cypress/**"
- "docker/datahub-frontend/**"
ingestion:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does it make sense to add an "ingestion-base" change detector?

we have a fair bit of logic around conditionally building the ingestion-base image, and I think it might make sense to refactor that into here or docker-unified's setup job

that said, this PR is already a huge improvement as it is and this is more of a nice to have and definitely not a blocker

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly I'll have to look at that logic again. There is an extra bit of complexity due to multiple stages plus full/slim. Since that was working ok as far as I know, I opted to just preserve that logic. In this PR it just kind of gets triggered all the time and its own logic maintains control over when and what to build.

echo 'matrix=["no_cypress_suite0","no_cypress_suite1","cypress_suite1","cypress_rest"]' >> $GITHUB_OUTPUT
else
echo 'matrix=[]' >> $GITHUB_OUTPUT
fi
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is going to be hugely helpful

@david-leifker david-leifker marked this pull request as ready for review January 20, 2024 02:49
@david-leifker david-leifker merged commit 087d3fd into datahub-project:master Jan 20, 2024
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devops PR or Issue related to DataHub backend & deployment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants