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

refactor: execute Python file scripts instead of in-line scripts #23

Merged
merged 38 commits into from
Oct 1, 2024

Conversation

mariajgrimaldi
Copy link
Collaborator

@mariajgrimaldi mariajgrimaldi commented Sep 28, 2024

Description

This PR is a continuation of a proposal I made here about making scripts more maintainable in the long term. I mentioned I had to figure out a good way of calling scripts within our workflow. Here's what I came up with:

Python scripts hosted in .github/workflows/scripts

The additional functionalities with complex logic for the workflow should be implemented in a Python script hosted in a subdirectory within .github/workflows/scripts with an explicit name expressing its purpose. In this case, the script gets tutor configuration values, so the Python script is called get_tutor_config.py. Each script should be well-documented and modular, have a single responsibility, and be able to be executed locally for easier debugging.

Call scripts from reusable workflow

Scripts should be executed from a new step in the workflow that passes the necessary arguments and handles the scripts' output.

For this reusable workflow to use files from its repo, we must implement an explicit step to check out the code since, by definition, reusable workflows check out code from the caller workflow repository: https://docs.github.com/en/actions/sharing-automations/reusing-workflows#overview

If you reuse a workflow from a different repository, any actions in the called workflow run as if they were part of the caller workflow. For example, if the called workflow uses actions/checkout, the action checks out the contents of the repository that hosts the caller workflow, not the called workflow.

When a reusable workflow is triggered by a caller workflow, the github context is always associated with the caller workflow. The called workflow is automatically granted access to github.token and secrets.GITHUB_TOKEN. For more information about the github context, see "Accessing contextual information about workflow runs."

This explains why we added an extra step here, and why we're now checking out code in subdirectories, so there's no overriding.

I consider this approach more maintainable since we can now easily lint, test, and debug scripts by removing the overhead of having hard-coded logic within the workflow. The scripts hosted in .github/workflows/scripts are meant to handle all complex logic to avoid returning to maintaining our previous approach.

How to test

  1. Go to edxn-strains repository where the caller workflow for Picasso is hosted: https://github.com/eduNEXT/ednx-strains/actions
  2. To manually trigger the ednx-strains/.github/build.yml workflow execution, go to Actions > Build Open edX strain, fill in the necessary configuration, please use the workflow version from MJG/scripts-folder. For the strain to build, I suggest using the MJG/scripts-folder strain branch to avoid overriding the current image on dockerhub. In this case, you don't need to wait for the workflow to finish to know it works, just check Get Tutor Configurations from config.yml and set them as an environment variable the step Get Tutor Configurations from config.yml and set them as an environment variable is successful.
    For a successful build, use MJG/scripts-success.
    For a build with missing configurations, use MJG/scripts-fail.
    For a build with a different registry (optional configuration) use MJG/support-diff-registries. To successfully push the image to Amazon ECR, you'll need to use the workflow from MJG/support-diff-registries as well.
  3. Press run workflow.

This implementation is not set in stone, so feedback is very welcome.

@mariajgrimaldi mariajgrimaldi marked this pull request as ready for review September 30, 2024 06:04
@mariajgrimaldi mariajgrimaldi requested a review from a team September 30, 2024 11:38
Copy link

@BryanttV BryanttV left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @mariajgrimaldi! Just a small comment

.github/workflows/scripts/get_tutor_config.py Outdated Show resolved Hide resolved
.github/workflows/build.yml Outdated Show resolved Hide resolved
@mariajgrimaldi mariajgrimaldi merged commit a3b4723 into main Oct 1, 2024
@mariajgrimaldi mariajgrimaldi deleted the MJG/scripts-folder branch October 1, 2024 09:22
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.

3 participants