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

Feature/upgrade adba to medusa adba #4822

Merged
merged 23 commits into from
Aug 18, 2018

Conversation

p0psicles
Copy link
Contributor

@p0psicles p0psicles commented Aug 1, 2018

  • PR is based on the DEVELOP branch
  • Don't send big changes all at once. Split up big PRs into multiple smaller PRs that are easier to manage and review
  • Read the contribution guide

Goal of this PR:

  • Use the refactored adba library located in pymedusa/adba. Which should be python3 compatible.
    But unfortunately it was not python 2 compatible, so I had to make some changes. From here we'll have to take another look at py3.
  • Make the adba lib accept a caching location. This way the xml files will be stored in medusa's cache location.
  • Removed adba "release group" code from the handler. This is going through api now. So we where hitting adba twice, whenever we opened the editShow page. On top of that, the apiv2 route, uses dogpile caching for anidb release groups. The handler doesn't.

Fixes #4820

The Py23ConfigParser class should be removed. And we should go for another solution.
The issue is this:
The included ConfigParser with python 2.7 does not work using the dict syntax on python2. You can use python3 syntax in python2 using the back ported configparser module available on PyPy.

So we could go with:
1. Add the backport configparser pypy version as a dependency in the adba lib.
2. Add the backport configparser version to our /ext folder
3. Transform adba lib to use python2 configparser syntax.

Libs: https://github.com/pymedusa/adba and https://github.com/pymedusa/simpleanidb have also been updated (develop branches)

* Removed release group code from editShow in handler.py. As we're using the apiv2 now. No need to get release groups twice from adba client.
@p0psicles
Copy link
Contributor Author

Linking for discussion #2439

@p0psicles
Copy link
Contributor Author

I'm a little lost on the ConfigParser thing. So I'd like to hear other opinions.

@p0psicles
Copy link
Contributor Author

@medariox?

Add package backports to /ext
Add package configparser to /ext
Update requirements.txt with configparser
Add makedirs, for cache folder, to lib adba.

These xml files conflict with one another.
use cache/adba for adba lib.
use cache/simpleanidb for simpleanidb lib.
@p0psicles p0psicles added Concluded Needs review Needs test (Python) Needs tests added for this change (Python) and removed In progress labels Aug 6, 2018
@@ -43,6 +41,8 @@ def anidb_logger(msg):
else:
return True
except Exception as error:
import traceback
print traceback.format_exc()
Copy link
Contributor

Choose a reason for hiding this comment

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

^

@p0psicles
Copy link
Contributor Author

Reminder to myself to also update changelog

resp = self.auth(self._username, self._password)
if resp.rescode not in ('500'):
if resp.rescode not in '500':
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be !=. Didn't review, just noticed while skipping through.

@p0psicles p0psicles closed this Aug 7, 2018
@p0psicles p0psicles reopened this Aug 8, 2018
@sharkykh sharkykh added Needs testing Requires testing to make sure it's working as intended and removed Needs test (Python) Needs tests added for this change (Python) labels Aug 9, 2018
@sharkykh
Copy link
Contributor

sharkykh commented Aug 9, 2018

You should push the latest change you made to adba to pymedusa/adba then update the commit hashes in requirements.txt and ext/readme.md (like I did)

@p0psicles
Copy link
Contributor Author

Pff. Yeah but then i need to make another change. Like to have approvals before i do that. I wont forget

@OmgImAlexis OmgImAlexis added this to the 0.2.9 milestone Aug 10, 2018
@p0psicles
Copy link
Contributor Author

Okay should be done now

requirements.txt Outdated
@@ -1,10 +1,13 @@
# No setup.py, unable to install using pip
git+https://github.com/pymedusa/adba.git@119b9d30a30feb97bcea5e1dc742c82340300091#egg=adba
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI, since the adba package has no setup.py, it can't be installed using pip, that's why I decided to comment it out and added a comment above.

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 yeah didnt think of that

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

Choose a reason for hiding this comment

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

Change the link text (inside the brackets) to the 7 first characters of the commit SHA (119b9d3)

Copy link
Contributor

@medariox medariox left a comment

Choose a reason for hiding this comment

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

LGTM

@p0psicles p0psicles merged commit d18ffa2 into develop Aug 18, 2018
@p0psicles p0psicles deleted the feature/upgrade-adba-to-medusa-adba branch August 18, 2018 20:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Concluded Needs review Needs testing Requires testing to make sure it's working as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants