ProjectPythia / projectpythia.github.io

https://projectpythia.org
Apache License 2.0
32 stars 17 forks source link

Artifact Passing from Automate-Metrics to Build-Book #415

Closed jukent closed 3 months ago

jukent commented 3 months ago

Editing this for conciseness for new readers:

This PR removes automatic metrics from trigger-preview and nightly-build. The metrics are only calculated in publish-book.yaml, which now happens weekly. We can have a wider conversation about the ideal relationship of each action.

Some changes were made to Cookbooks-action build-book.yaml (merged in other PRs) to update the infrastructure to handle the artifact passing in this case.


This PR attempts to check out the results of the directory after the automate-metrics step to be passed into the book-build action. Currently the repo is re-checked out and all liminal files are lost, resulting in no metrics on the website.

It looks like the book-build in Cookbook-actions already handles the logic for when an artifact is passed in:

    steps:
      - name: Checkout the code from the repo
        if: inputs.build_from_code_artifact == 'false'
        uses: actions/checkout@v4

      #  The next two steps should replicated checking out the code
      - name: Download code artifact
        id: get_code_artifact
        if: inputs.build_from_code_artifact == 'true'
        uses: dawidd6/action-download-artifact@v3.1.4
        with:
          github_token: ${{ secrets.GITHUB_TOKEN }}
          workflow: trigger-book-build.yaml
          run_id: ${{ github.event.workflow_run.id }}
          name: ${{ inputs.code_artifact_name }}

      - name: Unzip the code
        if: inputs.build_from_code_artifact == 'true'
        run: |
          unzip pr_code.zip
          rm -f pr_code.zip  

So this simply uploads the artifact and then grabs it later.

Related to https://github.com/ProjectPythia/projectpythia.github.io/issues/319

github-actions[bot] commented 3 months ago

👋 Thanks for opening this PR! The Cookbook will be automatically built with GitHub Actions. To see the status of your deployment, click below. 🔍 Git commit SHA: 5c468c4bfcfe6b1599ac97a9b0a066dfd1a85fdc ✅ Deployment Preview URL: https://projectpythia.github.io/_preview/415

erogluorhan commented 3 months ago

While I was checking this PR's files, I started to think that as a result of the recent automate metrics PRs, we made nightly-build.yaml and publish-site.yaml actions look and work very similar to each other, and we dropped the use cases where we might want to see effects of some implementations in the nightly builds without actually deploying them (e.g. how I did with the theme update recently). That said how about:

jukent commented 3 months ago

While I was checking this PR's files, I started to think that as a result of the recent automate metrics PRs, we made nightly-build.yaml and publish-site.yaml actions look and work very similar to each other, and we dropped the use cases where we might want to see effects of some implementations in the nightly builds without actually deploying them (e.g. how I did with the theme update recently). That said how about:

  • Revert the recent changes we made to the nightly-build as part of the metric automation work (i.e. remove the automate metrics and deploy the site jobs)
  • Instead add a nightly or weekly cron job into the publish-site.yaml so that the website can still be deployed automatically nightly/weekly (in addition to pushes to main and workflow-dispatch) with automated metrics

Thanks for the idea Orhan. It's good to know that there were use-cases for wanting to run the nightly build without deployment. I think the need for a nightly build is solved by running the publish-site weekly (and with fresh metrics). This PR now represents that.

jukent commented 3 months ago

YAML file is no longer malformed, and actions are running but failing at the build step during the downloading of the artifact.

Not Found - https://docs.github.com/rest/actions/workflow-runs#list-workflow-runs-for-a-workflow

There is a successful uploading of the artifact, but I'm not sure if I'm missing a critical zip step like this from the build-book action

      - name: Zip the book
        run: |
          set -x
          set -e
          if [ -f book.zip ]; then
              rm -rf book.zip
          fi
          zip -r book.zip ${{ inputs.path_to_notebooks }}/ ${{ inputs.output_path }}
jukent commented 3 months ago

One thing I'm noticing is that within the Cookbook-Actions book-build.yaml, if there is an artifact it runs like so:

      #  The next two steps should replicated checking out the code
      - name: Download code artifact
        id: get_code_artifact
        if: inputs.build_from_code_artifact == 'true'
        uses: dawidd6/action-download-artifact@v3.1.4
        with:
          github_token: ${{ secrets.GITHUB_TOKEN }}
          workflow: trigger-book-build.yaml
          run_id: ${{ github.event.workflow_run.id }}
          name: ${{ inputs.code_artifact_name }}

I think the action is failing on my fork because of workflow: trigger-book-build.yaml. There is not a trigger-book-build.yaml file on the Portal, so I am going to try to add one - but I'm a little confused about how it looks.

In the Foundations repo and all of the Cookbooks it looks like so:

name: trigger-book-build
on:
  pull_request:

jobs:
  build:
    uses: ProjectPythia/cookbook-actions/.github/workflows/build-book.yaml@main
    with:
      environment_name: pythia-book-dev
      artifact_name: book-zip-${{ github.event.number }}
      # Other input options are possible, see ProjectPythia/cookbook-actions/.github/workflows/build-book.yaml

This seems to work for Foundations and the Cookbooks, so editing build-book.yaml could break important things -- but I have to ask: Is this artifact call if-check being used anywhere? or tested?

It seems to me like this workflow would start the build-book.yaml from the top all over again. I'd edit it to include a code_artifact_name input (artifact_name points to where the book should be zipped, not where the zipped input is) and be triggered by a workflow_call instead of a PR. Still this new file will mostly be a place holder while I figure out what we want to do.

jukent commented 3 months ago

Thanks @brian-rose for explaining that it the workflow: trigger-book-build.yaml in Cookbook Actions build-book.yaml doesn't trigger the trigger-book-build.yaml workflow, but tells the book-build action to look in that action for the artifact.

I opened up a PR https://github.com/ProjectPythia/cookbook-actions/pull/104 that changes the trigger workflow to be an input with the previous default, that way we should be able to set it to publish-site.yaml in this workflow. I commented it out for now (because that input doesn't exist yet it stops the whole action from attempting to run with a YAML error)

jukent commented 3 months ago

Caught a typo in my Cookbook-actions update. I'm pretty sure that the trigger_workflow input should just be publish-book.yaml - but it could also need the full path .github/workflows/publish-book.yaml.

If I don't have success with either of those two - there seems to be way I could set the workflow to blank, and workflow_search to true. There are some options from the download-artifact documentation

jukent commented 3 months ago

Artifact passing is working on my branch! https://github.com/jukent/projectpythia.github.io/actions/runs/8425450722