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

Split parsing #788

Merged
merged 33 commits into from
Aug 3, 2016
Merged

Split parsing #788

merged 33 commits into from
Aug 3, 2016

Conversation

labrys
Copy link
Contributor

@labrys labrys commented Jul 19, 2016

  • 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 contribution guide

Separate the search and parsing functions in an effort to continue standardization of providers and enhancing of search functionality. This also subcategorizes the providers by type.

@labrys labrys added Enhancement Concluded Needs review Needs testing Requires testing to make sure it's working as intended labels Jul 19, 2016
@p0psicles
Copy link
Contributor

@labrys whats your plan of getting this in? As this can't be reviewed.
Only thing we can do, is make sure it's tested with the providers we have.

@duramato you can maybe give this a go for a week or so? Think that's the only way.
I'll remove the needs review, ast that's pretty much impossible.

@labrys labrys force-pushed the split-parsing branch 2 times, most recently from eb28292 to db455f2 Compare July 22, 2016 00:03
@fernandog
Copy link
Contributor

fernandog commented Jul 28, 2016

please respect rule!

  • Don't send big changes all at once. Split up big PRs into multiple smaller PRs that are easier to manage and review

+5,766 −4,939- Files changed 83


except (SocketTimeout, TypeError) as e:
Copy link
Contributor

@fernandog fernandog Jul 29, 2016

Choose a reason for hiding this comment

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

@medariox @labrys sockets timeout its not part of requests exception. you should known better

it will raise a traceback like this:

  File "C:\SickRage\Python\lib\httplib.py", line 832, in connect
    self.timeout, self.source_address)
  File "C:\SickRage\Python\lib\socket.py", line 575, in create_connection
    raise err
timeout: timed out

@medariox medariox added this to the 0.1.3 milestone Jul 29, 2016
@fernandog
Copy link
Contributor

need to change all logs from helpers.py

@fernandog fernandog force-pushed the split-parsing branch 2 times, most recently from a9a8580 to f2b2998 Compare July 30, 2016 12:22
@@ -1429,10 +1428,8 @@ def getURL(url, post_data=None, params=None, headers=None, # pylint:disable=too

try:
resp.raise_for_status()
except requests.RequestException as e:
except requests.exceptions.RequestException as e:
Copy link
Contributor

@fernandog fernandog Jul 30, 2016

Choose a reason for hiding this comment

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

@medariox shouldn't we add both? requests.RequestException and requests.exceptions ?

one exceptions is for requests itself. other is all other execeptions right?

like sockets, it's an exceptions but not part of Requests because its sockets

Copy link
Contributor

Choose a reason for hiding this comment

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

I think just requests.exceptions.RequestException should be fine.

@fernandog fernandog force-pushed the split-parsing branch 2 times, most recently from 6c75475 to 3ea784a Compare July 31, 2016 16:54
@fernandog fernandog modified the milestones: 0.1.3, 0.1.4 Jul 31, 2016
@@ -107,4 +106,5 @@ def getProviderClass(provider_id):
for x in sickbeard.providerList + sickbeard.newznabProviderList + sickbeard.torrentRssProviderList
if x
}
return providers[provider_id]
if providers:
return providers.values()[0]
Copy link
Contributor

@fernandog fernandog Aug 2, 2016

Choose a reason for hiding this comment

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

@medariox this change is making all providers in hstory appear with NZB image.

Also why getProviderClass used to return a list and now returns a dict?!
this change will need to be one in all over Medusa code!

Copy link
Contributor

Choose a reason for hiding this comment

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

Should be fixed now.
getProviderClass never returned a list, it always returned the first element of the list. So that's just the same, it's the first value of the dict. Since it makes very little sense to make a list/dict and return only the first element, I made it just directly return the first object now.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 28.874% when pulling 3d2a90a on split-parsing into cb3ec55 on develop.

@ratoaq2
Copy link
Contributor

ratoaq2 commented Aug 3, 2016

Approved

Approved with PullApprove

@medariox medariox merged commit 26c7e8a into develop Aug 3, 2016
@medariox medariox deleted the split-parsing branch August 3, 2016 19:31
@fernandog fernandog removed the Needs testing Requires testing to make sure it's working as intended label Feb 9, 2017
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.

None yet

6 participants