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 configure per show daily search airdate offset #5874

Merged
merged 10 commits into from
Dec 30, 2018

Conversation

p0psicles
Copy link
Contributor

@p0psicles p0psicles commented Dec 10, 2018

…ate offset.

  • Implemented check in dailsearch for searching episodes early.
  • 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

TODO:

  • add api spec
  • update changelog
  • add field to tv_shows db
  • add field to series class
  • add logic to dailsearch, for searching earlier
  • add logic to dailysearch, for search later
  • add UI control to editShow
  • add UI control to providers
  • add UI indicator to displayShow/snatchSelection header
  • Test by users

implement feature #3679

Update:

I've scratched the feature to enable/disable the feature per provider. As for that I need to make structural changes throughout medusa's code. And i'm not going to do that now.

…ate offset.

* Implemented check in dailsearch for searching episodes early.
@ghost
Copy link

ghost commented Dec 10, 2018

DeepCode analyzed this pull request.
There are no new issues.

@p0psicles p0psicles changed the title [WIP] Add new column airdate_offset to tv_shows table, to configure an aird… [WIP] Feature configure per show daily search airdate offset Dec 10, 2018
@rafi-d
Copy link
Contributor

rafi-d commented Dec 14, 2018

I've tested this with both per-search (negative offsets) and post-search (positive offsets), and everything seems to work OK!

Still, this offset needs to be visible on the show page, as well as the manual search page - in the top/header - show-info part, at least in cases it is not 0.

"UI control for providers" - in not necessary IMHO. Best keep it simple..

Thanks!

Rafi

#3679 (comment)

@rafi-d
Copy link
Contributor

rafi-d commented Dec 14, 2018

... one small typo in series.py -

"# The offset in ours, for when we want to start searching episodes for this show. -1 is search 1 hour early. 1, 1 hour later."

Also, be aware that once the early offset-time arrives, the status for a specific episode (changed to "wanted" when the offset time arrives) is not/cannot be reverted back to "unaired" even if the user corrects this offset, or sets it to 0 again. Not a big deal I believe, but it can be nice (and useful for testing) if there was a manual option to also change a status to 'unaired' ...

@p0psicles
Copy link
Contributor Author

... one small typo in series.py -
tnx, will correct

Also, be aware that once the early offset-time arrives, the status for a specific episode (changed to "wanted" when the offset time arrives) is not/cannot be reverted back to "unaired" even if the user corrects this offset, or sets it to 0 again. No a big deal I believe...

Yes I'm aware. Also, if the backlog search is triggered. It will also search for these early episodes if they're already set to wanted by the daily search.

@rafi-d
Copy link
Contributor

rafi-d commented Dec 14, 2018

This feature (dealing with offsets/time-limits) reminds of another desired feature - an autofiles size filter (min/max). Actually I have it working here locally great (tho, w/o changing anything in the DB yet). So, since you are changing the DB for this feature, it might be a good point in time to consider adding two more edit-boxes for this file-size feature if you are interested ... :)

@p0psicles
Copy link
Contributor Author

it might be a good point in time to consider adding two more edit-boxes for this file-size feature if you are interested ... :)

No i'm not interested. I already got like 6 other PR's on my backlog to finish. I did this one cause it's rather easy to implement.

@rafi-d
Copy link
Contributor

rafi-d commented Dec 14, 2018

Sure, no problem. Works for me...
For this feature here, I think you still need to add some "external" visibility for users .

@medariox
Copy link
Contributor

@rafi-d
We are aware of it. Once the PR is done we will label it as concluded.

@rafi-d
Copy link
Contributor

rafi-d commented Dec 14, 2018

@medariox
What I meant, was have the offset ( if set ) visible on the show's page among the other show's details, so to make it more clear why the status shows as "wanted" earlier or late relative to the air-time...

@p0psicles
Copy link
Contributor Author

Yes i updated the todo checklist of this pr.

@rafi-d
Copy link
Contributor

rafi-d commented Dec 15, 2018

1
Ok, Thanks. Actually these two are pretty much done IMHO... 👍
v add logic to dailysearch, for search later
v Test by users

One more thing to consider - one might see a strange show list like the attached (after a show has been snatched early, sorted by 'Next Ep', where future date(s) show in the 'Prev Ep' column). So, maybe adding another column - for "offset"- can be helpful here (disabled or enabled by default ) ?

Also, I'm missing a choice to change state to "un-aired", for both testing and fixing after testing...

@medariox medariox modified the milestones: 0.2.14, 0.2.15 Dec 19, 2018
@rafi-d
Copy link
Contributor

rafi-d commented Dec 25, 2018

I have been testing this for the last couple of weeks, and results are very good, but there is this one symptom which I've noticed, and not sure if it is related or not:
After an episode has been downloaded, say, -24 hours ahead of time as designed, I sometimes see an extra/second double download of the same episode for that same show with -24 offset, with a "Proper" notation, and it seems to occur after the original airtime. It also does not delete the previous downloaded episode (by uTorrent).
Is this something that can be expected? or maybe needs to be tweaked with regards to this new setting?

@p0psicles
Copy link
Contributor Author

Sounds like a "proper" was released after. But that should replace the original?

@rafi-d
Copy link
Contributor

rafi-d commented Dec 25, 2018

Yes, I guess it was released later on. And yes, if you decided to download same release twice, it should have deleted the first one. Honestly, I have never seen this happen.

Also, be aware that delete cleanly, is not that easy, since you have to remove it from the client (WebAPI to uTorrent) as well. So, in the worse case, if the client is still seeding it, you'd also need to stop it first, otherwise, the files might be locked (this was not the case here now).

@p0psicles
Copy link
Contributor Author

Okay but that doesn't sound as anything related to this pr. More something with your post-processing setup.

@rafi-d
Copy link
Contributor

rafi-d commented Dec 25, 2018

that doesn't sound as anything related to this pr

If so, let's forget it for now over here.

BTW, I do not have any post processing set, and didn't know that deleting of double download episodes even requires post-processing. I thought if it does an extra download, it should just clean/erase the previous one ....

I hope you release this feature soon, I am already one version behind... :)

@p0psicles
Copy link
Contributor Author

I'll merge develop into it.

@rafi-d
Copy link
Contributor

rafi-d commented Dec 25, 2018

maybe you can add minimal UI for it, so to make this easier to use/test?...

@ghost
Copy link

ghost commented Dec 25, 2018

DeepCode analyzed this pull request.
There are no new issues.

@rafi-d
Copy link
Contributor

rafi-d commented Dec 25, 2018

Got it. Looks good, thanks. I guess it is ready for release...

@rafi-d
Copy link
Contributor

rafi-d commented Dec 28, 2018

I guess "[WIP]" in the title is now redundant?...

@p0psicles
Copy link
Contributor Author

Yeah, just needs to get review

@medariox medariox modified the milestones: 0.2.15, 0.3.0 Dec 30, 2018
@medariox medariox changed the title [WIP] Feature configure per show daily search airdate offset Feature configure per show daily search airdate offset Dec 30, 2018
@medariox medariox merged commit 8676514 into develop Dec 30, 2018
@medariox medariox deleted the feature/add-daily-search-start-search-offset branch December 30, 2018 14:18
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.

3 participants