Skip to content
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 adba error handling #4961

Merged
merged 9 commits into from
Sep 4, 2018
Merged

Fix adba error handling #4961

merged 9 commits into from
Sep 4, 2018

Conversation

p0psicles
Copy link
Contributor

@p0psicles p0psicles commented Aug 25, 2018

The show How not to summon a demon lord fails to get release group info from anidb.
This is an anidb issue. As they are sending a bad response.

The problem was that 1. the adba lib, can't properly handle the UnicodeDecodeException.
And 2. the to_json of the Series class, was not handling the previously raised exception.

Note it still blocks the apiv2 tornado thread (which we currently only have 1) for a while. But after a few minutes it's given free again. The release groups retrieved (none), are cached in anidb.dbm, so you'll not have this error often (at least until cache expires).

Still need to have a proper solution for this.

@p0psicles

This comment has been minimized.

Required because an exception can be raised when trying to get the release groups for an anime show.
@sharkykh sharkykh added this to the 0.2.9 milestone Aug 25, 2018
@sharkykh sharkykh added the Bug label Aug 25, 2018
data['config']['release']['allgroups'] = get_release_groups_for_anime(self.name)
except AnidbAdbaConnectionException:
log.warning('An anidb adba exception occurred when attempting to get the release groups for the show {show}',
{'show': self.name})
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm thinking maybe return an empty list here if failed.

data['config']['release']['allgroups'] = []

data['config']['release']['allgroups'] = get_release_groups_for_anime(self.name)
try:
data['config']['release']['allgroups'] = get_release_groups_for_anime(self.name)
except AnidbAdbaConnectionException:
Copy link
Contributor

Choose a reason for hiding this comment

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

You don't need to log the error here? (as a debug log maybe)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought a warning is fitting. Then the user at least knows why its not getting any release groups when they are available in anidb.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

Return empty list in case of an exceptin.
@p0psicles
Copy link
Contributor Author

aah I need to update adba again of course. I'll add the changelog.

data['config']['release']['allgroups'] = []
log.debug(
'An anidb adba exception occurred when attempting to get the release groups for the show {show}',
{'show': self.name}
Copy link
Contributor

Choose a reason for hiding this comment

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

Well it's not really what I meant,
I suggested you add the actual error to the log...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ooh so make warning and add error?

ext/readme.md Outdated
@@ -1,7 +1,7 @@
## ext
Status | Package | Version / Commit | Usage | Notes
:------: | :-------: | :----------------: | :---- | :----
:: | `adba` | pymedusa/[119b9d3](https://github.com/pymedusa/adba/tree/119b9d30a30feb97bcea5e1dc742c82340300091) | **`medusa`** | -
:: | `adba` | pymedusa/[119b9d3](https://github.com/pymedusa/adba/tree/70bc381a75e20e1813c848c1edb7c6f16987397e) | **`medusa`** | -
Copy link
Contributor

@sharkykh sharkykh Aug 28, 2018

Choose a reason for hiding this comment

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

- pymedusa/[119b9d3](https://github.com/pymedusa/adba/tree/70bc381a75e20e1813c848c1edb7c6f16987397e)
+ pymedusa/[70bc381](https://github.com/pymedusa/adba/tree/70bc381a75e20e1813c848c1edb7c6f16987397e) 

Maybe I should just change the link to say pymedusa/[commit] (literally) instead, it would be easier to update...

@p0psicles
Copy link
Contributor Author

@sharkykh better?

@sharkykh
Copy link
Contributor

sharkykh commented Sep 4, 2018

We might have to revisit it later - as Series.to_json() is going to be called a lot. Maybe we don't call anidb unless the data is requested:
displayShow (?) / editShow need it,
but the home page doesn't need it to create the show list.

@sharkykh sharkykh changed the title Bugfix/adba error handling Fix adba error handling Sep 4, 2018
@p0psicles
Copy link
Contributor Author

Yeah thats in line with what i said before. That we should move this to a dedicated call.

@p0psicles p0psicles merged commit a587577 into develop Sep 4, 2018
@p0psicles p0psicles deleted the bugfix/adba-error-handling branch September 4, 2018 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants