Skip to content

NPOStart gives regularly 502 errors, handle those#2163

Merged
cvium merged 3 commits intoFlexget:developfrom
mfonville:develop
Jul 11, 2018
Merged

NPOStart gives regularly 502 errors, handle those#2163
cvium merged 3 commits intoFlexget:developfrom
mfonville:develop

Conversation

@mfonville
Copy link
Copy Markdown
Contributor

@mfonville mfonville commented Jul 3, 2018

increase the domain delay limit
do not raise a plugin-error on non-critical failed fetches

increase the domain delay limit
do not raise a plugin-error on non-critical failed fetches
'tiletype': 'asset'}

if not series_info:
while not series_info: # loop, so that if it fails it will retry
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is potentially an endless loop...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I will update the PR, instead of an endless loop, it will skip the series if it was not able to fetch its info

except RequestException as e:
raise plugin.PluginError('Request error: %s' % str(e))
log.error('Request error: %s' % str(e)) # if it fails, just go to next favourite

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If the error isn't an error, use log.warning

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ok, technically it is still an error of course; we just decide not to let the whole plugin die on this. But this series will not be fetched any further

Copy link
Copy Markdown
Contributor

@cvium cvium Jul 4, 2018

Choose a reason for hiding this comment

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

An error is something like unexpected behaviour. If the retry still fails after X tries and you stop the loop, then it's an error. Until then it's expected albeit not ideal behaviour.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

But this is not a loop, if it fails at this point, it is unexpected behaviour and it just stops further processing of this specific series. It moves on to the next series.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I didn't actually check the context, my bad. It could go either way for me wrt. log level.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Then I think I'd prefer to put in the error again; I think that is most understandable for the user (and the user probably would want to run the task again)

@mfonville
Copy link
Copy Markdown
Contributor Author

Are there any more changes desired before this could be merged?

@cvium cvium merged commit ca6317a into Flexget:develop Jul 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants