Better logging if writing a tag/category/author page fails#3266
Better logging if writing a tag/category/author page fails#3266minchinweb wants to merge 3 commits intogetpelican:mainfrom
Conversation
Because, slug is empty and depending on >>> from pelican.urlwrappers import Tag
>>> from pelican.settings import DEFAULT_CONFIG
>>> t = Tag('**', DEFAULT_CONFIG)
>>> t.slug
''And that's because Off the top of my head, I cannot think of any reason for wanting an empty slug intentionally. So if no objections, we could error for empty slug. |
|
I've updated it as per your comments! This is the current message: I have it throw a warning (rather than a critical error) if the slug is invalid, but throws the "regular" critical error if it's run into some other problem. I've "un- pre- formatted" the log messages, and dropped the Overall, it makes sense that slugify would strip out questionable character; I was just annoyed that Pelican would break but not really tell me what was wrong or how to fix it; I hope this explains how to fix the issue to the next user. |
e675806 to
4909657
Compare
|
I've added a warning when trying to generate a slug from a name that won't give a valid slug, and downgraded the former warning to an INFO message. So the displayed messages (when run with |
| if not tag.slug: | ||
| logger.info( | ||
| 'Tag "%s" has an invalid slug; skipping writing tag page...', | ||
| tag, | ||
| extra={"limit_msg": "Further tags with invalid slugs."}, | ||
| ) | ||
| continue |
There was a problem hiding this comment.
With empty slugs already generating warnings, is this special case necessary anymore? Why not just do the else block?
There was a problem hiding this comment.
I think it's helpful to still keep this, because it makes it very explicity what Pelican is (or isn't doing), and it's not entirely obvious that an invalid slug would keep the page from being generated.
Also, it mirrors the logging for all the pages that are written.
|
Hi @avaris ! This seems to have been lost in the shuffle. Do you want me to remove the logging (of the non-generated pages), or is it good to merge as is? Thanks! |
|
Hey @avaris ! Let me know if anything more is needed to merge this! Thanks! |
|
Apologies, offline life get in the way of things. I'll try to get back to this ASAP. |
No worries at all. My offline life has kept me from anything Pelican-related for the last couple months too... |
|
Hey @avaris , just wanted to check in on this. I'm maintaining a local fork with this on my side, but would love to just see it merged in.... Thanks for your help. |
|
Just wanted to bump this, hoping to get it merged! It's one of the patches I'm maintaining locally, and I was going to get up a new pull request till I remembered this is still here waiting... Let me know if changes are needed; I'm happy to make them. |
| tag, | ||
| extra={"limit_msg": "Further tags with invalid slugs."}, | ||
| ) | ||
| continue |
There was a problem hiding this comment.
I believe except RuntimeError will catch any RuntimeError — not just the file-overwrite error. Perhaps it might be better to catch a more specific exception. Maybe introduce a custom FileOverwriteError subclass of RuntimeError in writers.py? What do you think?
Add a short logging message if for some reason an invalid tag is provided, and Pelican instead tries to overwrite the index page.
Before:
After:
(For some reason, the tag
**is mapped onto the index page. Perhaps that is another issue that should be fixed...)Fix is applied when writing Tag, Category, and Author pages. Let me know if it should be added other places.
Pull Request Checklist