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

[Maint] fix the download and deploy part of workflow #380

Merged
merged 2 commits into from
Apr 5, 2024

Conversation

psobolewskiPhD
Copy link
Member

References and relevant issues

The second part, deployment, of the new unified workflow is failing:
Error: Unable to download artifact(s): Artifact not found for name: docs
https://github.com/napari/docs/actions/runs/8457634044/job/23170039408
If you check the timestamps it's running concurrently with the build&upload job, so the artifact probably isn't there to download.
Additionally, I think the publish_dir is wrong, because it's the dir the docs were built into, but now the docs are being downloaded as an artifact, so the artifact should be used.

Description

In this PR:

  • I make the 2nd job depend on the completion of the first using needs
  • I change the name to Download & Deploy to reduce confusion with the previous job.
  • I think the extra path in for the actions/download-artifact@v4 step is spurious, so I delete it.
  • fix the publish_dir of the deploy action to use the downloaded artifact

@psobolewskiPhD psobolewskiPhD requested a review from Czaki March 27, 2024 21:00
@github-actions github-actions bot added the task label Mar 27, 2024
@psobolewskiPhD psobolewskiPhD added maintenance CI, dependencies, and other maintenance priority-high and removed task labels Mar 27, 2024
@psobolewskiPhD psobolewskiPhD added this to the 0.5.0 milestone Mar 27, 2024
@psobolewskiPhD psobolewskiPhD changed the title fix the download and deploy part of workflow [Maint] fix the download and deploy part of workflow Mar 27, 2024
Copy link
Member

@melissawm melissawm left a comment

Choose a reason for hiding this comment

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

I think all of that is reasonable, but testing would take a fake deployment in a branch. I can try doing that tomorrow.

@melissawm
Copy link
Member

Ok - looking at napari/napari-sphinx-theme#160 I figured it out:

OK! That actually explains the failure: https://github.com/napari/docs/actions/runs/8462011401/job/23182687077

We don't use _build/html as an output for the docs build process in the Makefile:

docs/Makefile

Line 29 in 54ec184

docs-build: prep-docs

Meaning the artifact we are trying to upload here does not exist:

path: docs/docs/_build/html

So we need to update the workflow to upload _build (removing html), or we need to update the Makefile to build the docs to html. In that case, the CircleCI action will also need to be updated, as well as the contributing documentation guide.

@psobolewskiPhD
Copy link
Member Author

psobolewskiPhD commented Mar 28, 2024

We don't use _build/html as an output for the docs build process in the Makefile:

But I think it should still be the correct path to upload, because the docs-build in make uses the -M flag:

docs/Makefile

Lines 29 to 30 in 54ec184

docs-build: prep-docs
NAPARI_CONFIG="" NAPARI_APPLICATION_IPY_INTERACTIVE=0 sphinx-build -M html docs/ docs/_build -D sphinx_gallery_conf.examples_dirs=$(GALLERY_PATH) $(SPHINXOPTS)

This was changed in the unify-docs PR:
#348
So with -M, from the docs:

The default output directory locations when using make-mode differ from the defaults when using -b.

  • doctrees are saved to /doctrees
  • output files are saved to /

the outputdir is docs/_build so the html should end up in docs/_build/html (because html is the builder). Then we need .../html/ I think for the artifact upload.

In that Unify PR I also fixed the CircleCI workflow:

- store_artifacts:
path: docs/docs/_build/html/
- persist_to_workspace:
root: .
paths:
- docs/docs/_build/html/

So that works correctly -- you can see in the action that ran for this PR.
That gives me some confidence that this should work as well.

So the only question is setting the paths correct for the download here.
My understanding is that the download_artifact action will just unzip it to the workspace so the path structure won't be there as it would be when building in the same workflow step. That's why I changed the path in the deploy step.
Testing this is pretty hard though!

Copy link
Collaborator

@lucyleeow lucyleeow left a comment

Choose a reason for hiding this comment

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

LGTM from my limited knowledge. It doesn't seem like the trailing slash isn't strictly needed in:

path: docs/docs/_build/html/

https://github.com/actions/upload-artifact?tab=readme-ov-file#upload-an-entire-directory

but works fine either way. Thanks for fixing this!

@psobolewskiPhD psobolewskiPhD merged commit cdb5f2c into napari:main Apr 5, 2024
8 checks passed
@psobolewskiPhD psobolewskiPhD deleted the fix_deploy_workflow branch April 5, 2024 01:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance CI, dependencies, and other maintenance priority-high
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants