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

ci: Refactor the workflows #322

Merged
merged 7 commits into from
Sep 4, 2023
Merged

ci: Refactor the workflows #322

merged 7 commits into from
Sep 4, 2023

Conversation

ankatiyar
Copy link
Contributor

@ankatiyar ankatiyar commented Aug 31, 2023

Description

Development notes

Reusable workflows: unit-tests.yml, lint.yml, e2e-tests.yml -

Broke down check-plugin.yml which had a lot of case based jobs into reusable workflows -

  • unit-test.yml
  • lint.yml
  • e2e-tests.yml
    These workflows receive the python-version, os, and plugin as inputs from caller workflows.

Caller workflows: kedro-airflow.yml, kedro-datasets.yml, kedro-docker.yml, kedro-telemetry.yml

These workflows call the individual reusable workflows with the plugin, os, and python-version depending on which plugin needs which tests.

  • kedro-airflow : Calls unit-tests, lint, and e2e-tests.
  • kedro-datasets: Calls unit-tests, lint, and RTD-build(which is not a reusable workflow since it's only used by datasets)
  • kedro-docker: Calls unit-tests, lint, and e2e-tests.
  • kedro-telemetry: Calls unit-tests, lint, and e2e-tests.

NOTE: I've also made these workflows reusable so that all tests for a plugin can be run by simply invoking these workflows, e.g. in nightly-build.yml and check-release.yml

Release workflow: check-release.yml

Mostly left unchanged but -

  • Instead of a singular test job which is needed for the build-publish job, it now has airflow-test, datasets-test, docker-test, and telemetry-test which runs the whole test suite for the plugin that is being released.
  • As long as all the tests pass or are skipped, it proceeds as usual

NOTE: Tested on my fork by bumping telemetry version and removing the actual github release and PyPI publishing steps from the workflow. See the workflow file and the run.

Nightly build and notification: nightly-build.yml

This workflow also calls the reusable composite workflows kedro-airflow/datasets/docker/telemetry.yml. Runs everyday at midnight UTC.
For each plugin -

  • <plugin>-test is called.
  • The second job notify-plugin runs incase of a failure and creates a new issue with the label <plugin> nightly build if it doesn't exist/closed previously, or adds a comment to a pre-existing issue if it does.

NOTE: This creation of issue is based on the label not the issue heading.
NOTE: Tested on my fork by deliberately failing tests. See the created issues here - https://github.com/ankatiyar/kedro-plugins/issues

Checklist

  • Opened this PR as a 'Draft Pull Request' if it is work-in-progress
  • Updated the documentation to reflect the code changes
  • Added a description of this change in the relevant RELEASE.md file
  • Added tests to cover my changes

Signed-off-by: Ankita Katiyar <[email protected]>
Signed-off-by: Ankita Katiyar <[email protected]>
Signed-off-by: Ankita Katiyar <[email protected]>
@ankatiyar ankatiyar marked this pull request as ready for review September 4, 2023 10:55
Copy link
Member

@merelcht merelcht left a comment

Choose a reason for hiding this comment

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

This looks great, thanks @ankatiyar ! Awesome solution for the build failure notification through ticket creation 👍 Great description on the PR again as well ⭐

Does this help at all with #218? cc: @SajidAlamQB

if: ${{ needs.check-version.outputs.new_release == 'true' }}
needs: [ check-version, airflow-test, docker-test, datasets-test, telemetry-test ]
if: |
always() &&
Copy link
Member

Choose a reason for hiding this comment

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

What does always() do/mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This solution is from here -> actions/runner#491 (comment)
The build-publish job will still run if any of the needed tests are skipped (which they are if they're not due for a release)

telemetry-test:
uses: ./.github/workflows/kedro-telemetry.yml

notify-airflow:
Copy link
Member

Choose a reason for hiding this comment

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

Nice! 👍

@ankatiyar
Copy link
Contributor Author

It doesn't help with #218, for that we'd need to change the check-version job.
We could look into splitting the release workflows for each plugin too in the future.

Copy link
Contributor

@SajidAlamQB SajidAlamQB left a comment

Choose a reason for hiding this comment

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

This is some heroic work @ankatiyar! 🌟 The CI looks so much neater thank you for taking on this on and killing many birds with one stone!

@SajidAlamQB
Copy link
Contributor

Does this help at all with #218? cc: @SajidAlamQB

Not quite @merelcht we will have to change the github_actions_release.py script as that only triggers one release at a time.

@ankatiyar ankatiyar merged commit 7557498 into main Sep 4, 2023
51 checks passed
@ankatiyar ankatiyar deleted the refactor-ci branch September 4, 2023 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor GitHub Actions CI setup Add notifications for nightly build failures
3 participants