Pelican-Elegant / elegant

Best theme for Pelican Static Blog Generator
https://elegant.oncrashreboot.com/
MIT License
293 stars 187 forks source link

Pelican traceback with RSS Feeds generation #196

Closed iranzo closed 5 years ago

iranzo commented 5 years ago

Hi If I do set RSS feeds generation for anything other than ALL:

# CATEGORY_FEED_ATOM = 'feeds/{slug}.atom.xml'
# CATEGORY_FEED_RSS = 'feeds/{slug}.rss'
# TRANSLATION_FEED_ATOM = 'feeds/{lang}.atom.xml'
# TRANSLATION_FEED_RSS = 'feeds/{lang}.rss'
# AUTHOR_FEED_ATOM = 'feeds/{slug}.atom.xml'
# AUTHOR_FEED_RSS = 'feeds/{slug}.rss'
# TAG_FEED_ATOM = 'feeds/tag_{slug}.atom.xml'
# TAG_FEED_RSS = 'feeds/tag_{slug}.rss'

I do get a pelican traceback:

-> Writing /home/iranzo/DEVEL/private/www/output/feeds/all.atom.xml
-> Writing /home/iranzo/DEVEL/private/www/output/feeds/all.rss
CRITICAL: TypeError: not all arguments converted during string formatting
Traceback (most recent call last):
  File "/usr/bin/pelican", line 11, in <module>
    load_entry_point('pelican==3.7.1', 'console_scripts', 'pelican')()
  File "/usr/lib/python3.7/site-packages/pelican/__init__.py", line 487, in main
    pelican.run()
  File "/usr/lib/python3.7/site-packages/pelican/__init__.py", line 179, in run
    p.generate_output(writer)
  File "/usr/lib/python3.7/site-packages/pelican/generators.py", line 599, in generate_output
    self.generate_feeds(writer)
  File "/usr/lib/python3.7/site-packages/pelican/generators.py", line 312, in generate_feeds
    % cat.slug, feed_title=cat.name)
TypeError: not all arguments converted during string formatting
iranzo commented 5 years ago

Tried with pelican 4

Issue seems removed when removing in CATEGORY_FEED_ATOM|format(XXXX)

removing the |format part on the relevant category, etc makes issue go away

Can someone with more experience review implications to see if we should remove them?

silverhook commented 5 years ago

Feeds seem to work fine for me: http://matija.suklje.name/

I’m currently at tag 1.3 / commit 7ec41570b6ba4d6832146e98b05a42066abd2511 (sad, I know; I really need to update). So if this stopped working, it’s a regression. OK, I updated to latest master / 8a0bde0de7d1e40a38f61527576262d83cacde5b and it still works.

These are my feed settings:

# Feeds
FEED_ALL_ATOM = 'feeds/all.atom.xml'
CATEGORY_FEED_ATOM = 'feeds/%s.atom.xml'
TAG_FEED_ATOM = 'feeds/%s.atom.xml'
TRANSLATION_FEED_ATOM = None
iranzo commented 5 years ago

With latest pelican (4.01) I do get %s as obsolete and use relevant slug

In my case it was latest pelican + latest elegant and got something similar with blueidea theme too

silverhook commented 5 years ago

I’m on Pelican 3.7.1 (Debian stable). Perhaps that’s the issue?

AWegnerGitHub commented 5 years ago

The correct way to fix this is to change these two: (found in /templates/_includes/feeds_categories.html)

{{ CATEGORY_FEED_ATOM|format(cat_name) }}
{{ CATEGORY_FEED_RSS|format(cat_name) }}

to these:

{{ CATEGORY_FEED_ATOM|format(slug=cat_name) }}
{{ CATEGORY_FEED_RSS|format(slug=cat_name) }}

This is mentioned in the 4.0 release docs: here

Some user-submitted themes use positional argument formatting on object-related feed URLs, which will cause sites to fail to build with: "TypeError: not all arguments converted during string formatting". In that case, the theme needs to be updated. For example, substitute TAG_FEED_ATOM|format(tag.slug) with TAG_FEED_ATOM|format(slug=tag.slug).

silverhook commented 5 years ago

@AWegnerGitHub So, this is new in Pelican 4.0?

talha131 commented 5 years ago

@AWegnerGitHub how does this change effect Pelican version 3.? Because if it breaks 3. then we should put this fix in "release 2.1" (which is about Pelican 4.0 support) and not 2.0.

AWegnerGitHub commented 5 years ago

This is Pelican 4.0 behavior (along with the switch from %s to {slug}). I am not in a position to test for several more hours on how the 3.7 vs 4.0 behavior is different. I suspect this may be something that is specific to pelican 4.0 though because of the change to using .format() parameters

iranzo commented 5 years ago

slug=cat_name

Seems that pelican 4.0.1 works fine with above change, how should this be addressed (based that our documentation is pulling from latest pelican and latest master for the build)?

iranzo commented 5 years ago

Updated https://github.com/Pelican-Elegant/elegant/pull/197 to use the 4.0 approach

iranzo commented 5 years ago

@AWegnerGitHub @silverhook can you review the changes?

If so, can you test it on pelican 3.7 or others to see if it works for you and can be added to master?

AWegnerGitHub commented 5 years ago

This approach does not work with 3.7.1 if we've converted the configuration line to use CATEGORY_FEED_ATOM = 'feeds/{slug}.atom.xml'. It still fails with:

Traceback (most recent call last):
  File "/home/andy/.virtualenvs/pelican_test/bin/pelican", line 11, in <module>
    sys.exit(main())
  File "/home/andy/.virtualenvs/pelican_test/lib/python3.6/site-packages/pelican/__init__.py", line 487, in main
    pelican.run()
  File "/home/andy/.virtualenvs/pelican_test/lib/python3.6/site-packages/pelican/__init__.py", line 179, in run
    p.generate_output(writer)
  File "/home/andy/.virtualenvs/pelican_test/lib/python3.6/site-packages/pelican/generators.py", line 599, in generate_output
    self.generate_feeds(writer)
  File "/home/andy/.virtualenvs/pelican_test/lib/python3.6/site-packages/pelican/generators.py", line 312, in generate_feeds
    % cat.slug, feed_title=cat.name)
TypeError: not all arguments converted during string formatting

The problem is how the two version are doing string formatting.


This does work with 4.0.1 and the CATEGORY_FEED_ATOM = 'feeds/{slug}.atom.xml' configuration entry.


I think we should hold off on merging this change until we support 4.0.x and make it explicit in our documentation that 4.0 support is coming. In that case, our documentation repository needs to be pinned to Pelican 3.7.1.

Elegant 2.0 should be ready "soon". If 2.1 is going to be getting us Pelican 4.0 compatibility, it isn't that far behind.

iranzo commented 5 years ago

Not sure if templates can contain code for 'both' releases using one or the other, but that could be a mid-way, having both 'ifs' and use one approach or the other depending on pelican version

iranzo commented 5 years ago

https://stackoverflow.com/questions/53363848/how-to-get-the-pelican-version-number-from-a-template and make that a requirement in pelicanconf for elegant and 'default' to '3.x' mode ?

talha131 commented 5 years ago

Thanks a lot @AWegnerGitHub for taking the time to test with old version.

@iranzo That's smart. But lets not do that. I am sure most users will upgrade to Pelican 4. And if some user holds back, the can use our 2.0 release. (We will use tags for releases, users will not any problem in downloading an older release of Elegant.)

The only reason we are having this discussion is because we need to release 2.0 ASAP, a goal which is not far away, thanks to generous contributions of you guys.

talha131 commented 5 years ago

Close via b2397e8f9d2095952f98bcd83685ef33ec6e43fd

talha131 commented 5 years ago

@AWegnerGitHub you have been contributing regularly, since the project management has changed. Is it ok if we add you as member of the organization?

What it entails is that we would be able to assign issues to you, request you to review PR. You can continue working at your own pace, there is no rush or deadlines. But we would know @AWegnerGitHub is responsible for so and so issue/review and he will eventually get back to us on it. This would be a great help for all of us.

@silverhook @iranzo and @calfzhou feel free to chime in.

iranzo commented 5 years ago

+1 from me

AWegnerGitHub commented 5 years ago

@talha131 That sounds good. I can provide reviews as needed and continue to contribute as I have been.

talha131 commented 5 years ago

Appreciated! I have invited you. Let me know if you didn't receive the invitation.

talha131 commented 5 years ago

You can also visit the following link to accept the invitation.

https://github.com/Pelican-Elegant

silverhook commented 5 years ago

A belated +1 from me too.