helpers icon indicating copy to clipboard operation
helpers copied to clipboard

Factor out redundancy in GH actions

Open gpsaggese opened this issue 9 months ago • 1 comments

We can't use our link approach so we want to use https://github.com/causify-ai/helpers/pull/374#issuecomment-2730958270

As an alternative, we can explore the reusable workflows approach as done in https://github.com/causify-ai/template/pull/2/files

  • [ ] Let's document this limitation / explain the problem and the solution in the documentation
  • [ ] Make a proposal of how the reusable approach works
  • [ ] It might be worth to add a line in our dev system paper since we want to describe how the composable GH workflows work

FYI @heanhsok @samarth9008 @Shaunak01

gpsaggese avatar Mar 17 '25 22:03 gpsaggese

Made https://github.com/causify-ai/cmamp/pull/11617 @samarth9008 Please let me know about the priority of this task.

dremdem avatar Mar 18 '25 11:03 dremdem

After analyzing the GH workflows and their steps, I came up with the first two reusable workflows:

  • Prepare Environment

    • Configure AWS credentials
    • Log in to GHCR
    • Cleanup
    • Checkout code
    • Update PYTHONPATH
    • Install dependencies
    • Pull image from GHCR
  • Run Tests

    • Call Prepare Environment
    • Run tests
    • Post status if triggered manually
    • Send Slack notification on failure

Both workflows can/should be parameterized.

For the first try, I'll go with the Run Tests GH workflow, parameterize it, and call it from the super-slim new run fast tests.

dremdem avatar Mar 31 '25 15:03 dremdem

For the first try, I'll go with the Run Tests GH workflow, parameterize it, and call it from the super-slim new run fast tests.

Made draft PR: https://github.com/causify-ai/helpers/pull/453

Test run: https://github.com/causify-ai/helpers/actions/runs/14176749164/job/39713376654?pr=453

It works well.

@samarth9008 Let me know if you agreed with the approach then I'll continue to the Prepare Environment reusable workflow.

Probably, need to test everything locally, in this repo and then move all the reusable workflow to the templates repo.

dremdem avatar Mar 31 '25 16:03 dremdem

@dremdem

What I would do is

  1. Create a common workflow for fast, slow and superslow tests which will do the following things
  • Configure AWS credentials
  • Log in to GHCR
  • Cleanup
  • Checkout code
  • Update PYTHONPATH
  • Install dependencies
  • Pull image from GHCR
  • Run tests
  • Post status if triggered manually
  • Send Slack notification on failure

The tests to run will be defined based on the variable from the yml files of fast, slow and superslow tests

  1. For linter, we can create a similar re-usable workflow and use that for all the linter runs across repos.
  2. Same approach as used in linters for update helpers submodule

For now, lets come-up with the draft PR for fast, slow and superslow tests

samarth9008 avatar Mar 31 '25 17:03 samarth9008

The tests to run will be defined based on the variable from the yml files of fast, slow and superslow tests

Can you please explain a little bit more about it? I'm not get it tbh.

What I did in the #453:

  • made reusable workflow tests.yaml with the parameter test-name
  • made the test_reuse_fast_tests.yml workflow that just call the test tests.yaml with the:
    • test-name: run_fast_tests

With this approach, we only need to use a single tests.yaml workflow across all repositories for all tests. Yes, we still keep files like fast_tests.yml, but it will only contain the following:

name: Test Reusable Fast tests
on:
  # Run on every PR to master that is ready to review (i.e., not draft).
  pull_request:
    # https://github.community/t/dont-run-actions-on-draft-pull-requests/16817/19
    types: [opened, synchronize, reopened, ready_for_review]
    branches:
      - master    
  workflow_dispatch:
concurrency:
  group: ${{ github.workflow }}-${{ github.ref }}
  cancel-in-progress: true
jobs:
  run_fast_tests:
    uses: ./.github/workflows/tests.yml
    with:
      test-name: run_fast_tests
    secrets: inherit

dremdem avatar Mar 31 '25 17:03 dremdem

What you did is correct. What I am unable to understand is why there are 2 re-usable workflows? The prepare env and run tests. What I am suggesting is there will be a common workflow that will do the following steps

  • Configure AWS credentials
  • Log in to GHCR
  • Cleanup
  • Checkout code
  • Update PYTHONPATH
  • Install dependencies
  • Pull image from GHCR
  • Run tests
  • Post status if triggered manually
  • Send Slack notification on failure

and based on your parameter test-name, the fast, slow and superslow tests will be executed from their respective files and will call the common re-usable workflow. For now, if its hard to understand, lets do a end-to-end draft PR for fast tests with your approach and we can discuss on the code.

samarth9008 avatar Mar 31 '25 17:03 samarth9008

Let's document the rationale / thinking in our docs, explaining the high level logic, limitations of the reusable workflows, etc.

gpsaggese avatar Apr 01 '25 22:04 gpsaggese

@dremdem

Could you point to the doc that was merged recently? I believe it has all the info that's mentioned in the above comment https://github.com/causify-ai/helpers/issues/384#issuecomment-2770808785

samarth9008 avatar Apr 02 '25 00:04 samarth9008

@dremdem

Could you point to the doc that was merged recently? I believe it has all the info that's mentioned in the above comment #384 (comment)

Here is the doc: https://github.com/causify-ai/cmamp/blob/master/docs/build/ck.github_actions.explanation.md#reusable-workflows

Probably need to add:

explaining the high level logic, limitations of the reusable workflows, etc.

dremdem avatar Apr 02 '25 09:04 dremdem

Follow-ups filed. Doc work is handled in this PR - https://github.com/causify-ai/cmamp/pull/11757

Helpers fast/slow/superslow are using refactored workflow

Closing the issue now.

samarth9008 avatar Apr 03 '25 19:04 samarth9008