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 not download new quality after quality change #887

Closed
wants to merge 11 commits into from

Conversation

fernandog
Copy link
Contributor

@fernandog fernandog commented Aug 13, 2016

as discussed in slack

Wanted quality is HD720P . and i have a SDTV when my show had SDTV wanted quality

From logs:
Episode already exists with quality SDTV and the found result has same/lower quality, ignoring found result for Ripper Street S04E03 with quality 720p WEB-DL

medusa ignored 720p result

image

@fernandog fernandog added Bug Needs testing Requires testing to make sure it's working as intended Do not merge labels Aug 13, 2016
@medariox medariox added this to the 0.1.4 milestone Aug 13, 2016
@fernandog
Copy link
Contributor Author

fernandog commented Aug 14, 2016

@ratoaq2 @medariox need to review all the tests.

If you have qualities in the Preferred list set, any episode matching a quality in this list will not be replaced with a better quality. AKA it is a "best" match and is final.

If you ONLY have qualities in the Allowed list (Or have a preset selected other than custom) it will also stop on the first match in this list and be considered a "best" match, and never be replaced with a better quality.

If you have qualities in BOTH lists, it will accept any quality in the Allowed list, and continue looking for a better quality in the Allowed list, until it finds one in the Preferred list.

@fernandog
Copy link
Contributor Author

fernandog commented Aug 14, 2016

@fernandog
Copy link
Contributor Author

fernandog commented Aug 14, 2016

This one is reladed to backlog overview

All logic should be in one place only right?
https://github.com/pymedusa/SickRage/blob/develop/sickbeard/tv.py#L1574

https://github.com/pymedusa/Medusa/blob/develop/medusa/tv.py#L1692-L1730

@fernandog
Copy link
Contributor Author

@medariox milestone 0.1.5 right? we need make sure this works fine (keep some days in develop)

@fernandog fernandog modified the milestones: 0.1.4, 0.1.5 Aug 21, 2016
@ratoaq2 ratoaq2 modified the milestones: 0.1.6, 0.1.5 Sep 12, 2016
@ratoaq2 ratoaq2 force-pushed the fix_change_quality branch 3 times, most recently from 4f6d348 to a9cac11 Compare September 19, 2016 16:35
@fernandog fernandog force-pushed the fix_change_quality branch 5 times, most recently from 0e5a0d6 to ac6c9f9 Compare October 6, 2016 18:08
@fernandog fernandog modified the milestones: 0.1.6, 0.1.7 Oct 7, 2016
@fernandog
Copy link
Contributor Author

My review about tests is missing? what happened?

need to add tests with status ARCHIVED and status UNKNOWN

@fernandog
Copy link
Contributor Author

@ratoaq2 @medariox there is too place that still does some quality checks:

https://github.com/pymedusa/Medusa/blob/develop/medusa/search/core.py#L323
https://github.com/pymedusa/Medusa/blob/develop/medusa/tv.py#L1692-L1730

all logic should be in one place right?

@NicoLeOca
Copy link

@fernandog
not sure it is ok for me to remind it here but:
can you please have a look to the fact that a manual snatch is directly considered as being of the prefered quality no matter what the exact quality is?
#872

the interest clearly depends on the ETA of this:
#1266
and if you really review it completely as @OmgImAlexis and I suggested.

@medariox
Copy link
Contributor

medariox commented Nov 5, 2016

Here is how it works

If you only have qualities in the Allowed list set (or have a preset selected other than custom), any episode matching a quality in this list will not be replaced with a better quality - it will stop at the first quality found. [unchanged behavior]

If you only have qualities in the Preferred list set it will accept any quality in the Preferred list, and continue looking for a better quality in the Preferred list until the highest quality is found. [changed behavior - previously it would be like "only qualities in Allowed list"]

If you have qualities in both lists, it will accept any quality in the Preferred or Allowed list, and continue looking for a better quality in the Allowed list, until it finds one in the Preferred list [unchanged behavior - logic is inverted here compared to 1st and 2nd scenario]

@fernandog
Copy link
Contributor Author

@medariox today this "If you only have qualities in the Preferred list set" is in allowed. are we changing it? users will need to change their quality settings?

@NicoLeOca
Copy link

@medariox
I didn't know that
that's a very good news then

Cheers

@medariox
Copy link
Contributor

medariox commented Nov 5, 2016

@fernandog
Users that have custom preset with only allowed qualities set will eventually need to change that to preferred qualities, in order to keep the same behavior.

@medariox medariox closed this Nov 5, 2016
@medariox medariox reopened this Nov 5, 2016
@ratoaq2
Copy link
Contributor

ratoaq2 commented Nov 5, 2016

Just need to invert the logic in the method and its test

@fernandog
Copy link
Contributor Author

superseeded

@fernandog fernandog closed this Nov 5, 2016
@fernandog fernandog deleted the fix_change_quality branch November 5, 2016 20:12
@fernandog fernandog restored the fix_change_quality branch November 5, 2016 20:13
@NicoLeOca
Copy link

@medariox @ratoaq2 @fernandog
Guys,
can you please add the explanations of how to handle quality settings somewhere easy to find for everyone?
like next to the settings
image

cause I use Medusa/SR for some time now and I just learned that if there are several prefered qualities setup, they will be downloaded successively up to the best.

@medariox medariox deleted the fix_change_quality branch November 23, 2016 11:44
@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

4 participants