fastai / fastpages

An easy to use blogging platform, with enhanced support for Jupyter Notebooks.
https://fastpages.fast.ai/
Apache License 2.0
3.53k stars 756 forks source link

fix yaml.load in upgrade action #597

Closed stefanbschneider closed 2 years ago

stefanbschneider commented 2 years ago

Currently, the upgrade action fails for me with the following error:

Run import re, os, yaml
Traceback (most recent call last):
  File "/home/runner/work/_temp/7a47792d-0b69-47f9-9572-afc4337b8a1d.py", line 22, in <module>
    config = yaml.load(cfg)
TypeError: load() missing 1 required positional argument: 'Loader'
Error: Process completed with exit code 1.

See the broken GitHub action in my blog: https://github.com/stefanbschneider/blog/runs/4387836262?check_suite_focus=true#step:7:40

According to the PyYaml docs, we need to specify a loader: https://pyyaml.org/wiki/PyYAMLDocumentation That's what I did in this PR.

hamelsmu commented 2 years ago

Thanks @stefanbschneider I fixed this in https://github.com/fastai/fastpages/commit/028122f9929d3fed7960ae80ac9dd68c24a165ba

However, this is a key weakness of fastpages, is I can't easily propagate a fix in the upgrade.yaml to users, because that is the yaml that controls propagating changes itself (its kind of meta).

But either what you did or pinning the version like I did will work, I might recommend pinning the version just to be safe.

Thanks again for the PR. I will close this, and will try to figure out if there is a way I can push a fix out to all users. Perhaps I can automatically open PRs en-masse using the GitHub API

stefanbschneider commented 2 years ago

Ah ok, I see. Is there any way, I can fix the broken upgrade action for my blog then? @hamelsmu

I was trying to upgrade to the latest fastpages version, partly because I am facing performance and usability issues on my blog (reported by Google Search Console & Analytics) and was hoping that the upgrade could solve it. But maybe that's a different issue.

hamelsmu commented 2 years ago

Yes the best way to fix this on your blog is to make the same change I did and pin the version. Let me know if that works

hamelsmu commented 2 years ago

This is temporary until I can figure out a creative way to fix this for everyone

stefanbschneider commented 2 years ago

Yes the best way to fix this on your blog is to make the same change I did and pin the version. Let me know if that works

It did work fine, thanks!

This is temporary until I can figure out a creative way to fix this for everyone

Instead of specifying requirements (here, pyyaml) in the upgrade.yaml directly, maybe you could have them in a separate upgrade_requirements.txt that you fetch along with the other files from the fastpages repo during upgrade - then, if requirements change again, you could just change upgrade_requirements.txt and running pip install -r upgrade_requirements.txt should use the new requirements.

Of course, this does not help with the current issue; just and idea for the future. But maybe there's a more elegant solution.