Skip to content

sonarr list refactor#2047

Merged
liiight merged 5 commits intodevelopfrom
sonarr_list_refactor
Dec 21, 2017
Merged

sonarr list refactor#2047
liiight merged 5 commits intodevelopfrom
sonarr_list_refactor

Conversation

@liiight
Copy link
Copy Markdown
Member

@liiight liiight commented Dec 20, 2017

Motivation for changes:

sonarr_list code was old and sucky. Also a bugfix

Detailed changes:

  • Added ability to bypass filters when listing all entries. It was needed when adding or removing a show in order to get the unfiltered list of shows
  • Made base_url default to 'http://localhost and made it not required
  • Refactor and tweaks

Addressed issues:

…ing filters, when relevant. Also refactor. Closes #2041
@liiight liiight requested a review from cvium December 20, 2017 14:38
url = '{}:{}/api/{}'.format(base_url, port, endpoint)
headers = {'X-Api-Key': self.config['api_key']}
if term:
url += '?term={}'.format(term)
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.

Two different ways of concatenating strings in the same line of code. Looks a bit odd, but I suppose it's irrelevant.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I had no choice with that. I had to build url like that since the param can contain :which requests escapes and sonarr doesn't like

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I just now get what you meant. yeah, it's kinda odd but i think it's clear, so...

else:
raise plugin.PluginError('Invalid response received from Sonarr: %s' % response.content)
rsp = requests.request(method, url, headers=headers, json=data)
rsp.raise_for_status()
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.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Dunno. Since I didn't need to use our custom session features I saw no reason using our own implementation

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.

It's just a wrapper. I'd rather we were consistent on the matter. You can just use this https://github.com/Flexget/Flexget/blob/develop/flexget/utils/requests.py#L253

if show['status'] == 'ended' and not self.config.get('include_ended'):
continue
if self.config.get('include_data') and profiles_json: # Check if to retrieve quality & path
if profiles: # Check if to retrieve quality & path
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.

Why not put the loop here instead?

else:
entry['configure_series_quality'] = fg_qualities
if path:
entry['configure_series_path'] = path
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.

Why not just put show.get('path') here?

@liiight liiight merged commit 89cbf05 into develop Dec 21, 2017
@liiight liiight deleted the sonarr_list_refactor branch December 24, 2017 10:03
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.

BUG: Invalid response received from Sonarr: This series has already been added

2 participants