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

Fix #1954 Cause RuntimeError if plugin throw StopIteration #1955

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@pluser

pluser commented Aug 20, 2015

Make Nikola raise RuntimeError If plugin throw StopIteration exception.
See the #1954 for detail.

@Kwpolska

This comment has been minimized.

Show comment
Hide comment
@Kwpolska

Kwpolska Aug 20, 2015

Member

Why bother? I would call this user error and not even following basic APIs because the function must return dicts

👎

Member

Kwpolska commented Aug 20, 2015

Why bother? I would call this user error and not even following basic APIs because the function must return dicts

👎

@pluser

This comment has been minimized.

Show comment
Hide comment
@pluser

pluser Aug 20, 2015

Yes. That is user error. Of course, user should return dicts.
However, If user mistake and raise exception, It is better to show kind error message (like Traceback) instead of ambiguous message like: Scanning posts...Scanning posts...Scanning posts...Scanning posts...Scanning posts...Scanning posts...Scanning posts...ERROR: render_site. Task dependency 'generate_rss' does not exist..
Would you agree with this view?

pluser commented Aug 20, 2015

Yes. That is user error. Of course, user should return dicts.
However, If user mistake and raise exception, It is better to show kind error message (like Traceback) instead of ambiguous message like: Scanning posts...Scanning posts...Scanning posts...Scanning posts...Scanning posts...Scanning posts...Scanning posts...ERROR: render_site. Task dependency 'generate_rss' does not exist..
Would you agree with this view?

@ralsina

This comment has been minimized.

Show comment
Hide comment
@ralsina

ralsina Aug 20, 2015

Member

It's early and I have had no coffee yet, but read_metadata can raise any other exception, and then the exact same thing will happen, right? This would only fix one specific problem.

Member

ralsina commented Aug 20, 2015

It's early and I have had no coffee yet, but read_metadata can raise any other exception, and then the exact same thing will happen, right? This would only fix one specific problem.

@pluser

This comment has been minimized.

Show comment
Hide comment
@pluser

pluser Aug 20, 2015

No. read_metadata() can raise other exception like RuntimeError, KeyError, NameError, or something, but If those were raised, Nikola show Traceback correctly. Only StopIteration make false report as far as I know.

pluser commented Aug 20, 2015

No. read_metadata() can raise other exception like RuntimeError, KeyError, NameError, or something, but If those were raised, Nikola show Traceback correctly. Only StopIteration make false report as far as I know.

@Kwpolska

This comment has been minimized.

Show comment
Hide comment
@Kwpolska

Kwpolska Aug 22, 2015

Member

@ralsina, opinions?

Member

Kwpolska commented Aug 22, 2015

@ralsina, opinions?

@ralsina

This comment has been minimized.

Show comment
Hide comment
@ralsina

ralsina Aug 22, 2015

Member

@Kwpolska I'll have to take a real look at the code that calls that. I suspect it's not a problem with every plugin but with task plugins because we are unrolling iterators, so StopIteration makes everyone go nuts.

Member

ralsina commented Aug 22, 2015

@Kwpolska I'll have to take a real look at the code that calls that. I suspect it's not a problem with every plugin but with task plugins because we are unrolling iterators, so StopIteration makes everyone go nuts.

@Kwpolska Kwpolska closed this Sep 5, 2015

@Kwpolska Kwpolska added the wontfix label Sep 5, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment