Ignore Failed Downloads #329

Closed
wants to merge 23 commits into
from

Projects

None yet

9 participants

@TFenby

Okay, it now works using info directly from SABnzbd, and should be 100% reliable.
Anything else I should add? Failed episode status? Configurable immediate retry?

Edit: I figure I should include the original description here.

So, here's a patch that tracks in the history database when a download fails (based on info passed to the sabnzbd user script). There's a setting on the post-processing page that determines whether or not failed downloads are deleted after being marked. Downloads that fail are re-set to Wanted.

Tyler Fenby added some commits Mar 6, 2012
Tyler Fenby Ignore failed downloads
Post processing a folder w/ _FAILED_ will set that nzb as failed in
history, and future searches will ignore it.
Todo:
* Way to manage/reset the list
* Configuration option to delete failed folder
c2aa3ed
Tyler Fenby Option to delete failed download
Added the option on the post-processing page to delete the folder of a
failed download during post-processing.
Additionally, fixed some except statements that had multiple exceptions
without parens.
5d3da51
Tyler Fenby Fix settings page
Didn't have delete_failed in the callback, so settings wouldn't save.
5a293df
Tyler Fenby Improve failed matching
Use SABnzbd's built-in nzb name/status passing to give us the exact nzb name
that failed. Does hella offer this capability?
06d2255
Tyler Fenby sabToSickBeard.py - fix argv comparison to int (should be str)
Was erroneously comparing argv[7] to an integer instead of a string.
9d6f07a
Tyler Fenby Missed a dir -> dirName conversion
Missed changing dir to dirName in autoProcessTV.py
57d9c65
Tyler Fenby Proper handling of types w/ cherrypy
Probably shouldn't have expected a boolean to pass through as a boolean.
47aefcb
Tyler Fenby Manually mark as failed in post processing
Added checkbox to manual post processing page to allow users to manually
mark a download as failed.
c4a8878
@TFenby

I want to have a failed download be marked as wanted (instead of snatched), but setStatus is in webserve.py, which imports processTV.py. Do you care if there's a circular import between the two?

@midgetspy
Owner

I haven't had a chance to look at this whole thing but you definitely shouldn't need to use setStatus from webserve.py. In the post processing you probably already have the ep_obj (or can get it easily) and can just set the status on the episode object directly.

Also it looks like you added a failed column to the history database but I don't see a database migration?

@TFenby

Would you want me to use an ep_obj there, though? That'd be two places where episode status is changed, and I'd basically be copying and pasting the body of setStatus into the failed handling.

The migration is there in mainDB.py

Tyler Fenby added some commits Mar 11, 2012
Tyler Fenby Wrap str calls with ek.ek 89102da
Tyler Fenby Faster DB upgrade
Let SQLite handle it.
86e18b6
Tyler Fenby Set status from within processTV
Use own function in processTV, instead of call to existing function in
webserve
3149e00
Tyler Fenby Minor - Missed a returnStr 3d73acc
@smidley

I have a concern - If the download fails and is marked again as wanted, won't sickbeard just try to download the same episode again right away? If you could make sickbeard look for a different version (like how couchpotato works), I think it would be a lot better.

@TFenby

Er, that's the entire point of this patch.

@smidley

Ok, I wasn't sure if this patch just re-added the nzb again or if it actually let you tell it to check for a different version. Thanks. :)

@TFenby

Anything else I need to change/add? This is working great right now.

@d248066

Sorry to ask a stupid question, how can i apply this patch? Also, will this becoming out in the main release? Have only started to use github.

@TFenby

You can just download my version of SickBeard (it's just vanilla with these changes made) at https://github.com/schitso/Sick-Beard/downloads

@d248066

Thank you for the info. Just noticed that you have a different version of Couchpotato, is there any major features with your version? If i find any bugs, how can i report them? Also, is your sick beard version automatically get updates from the master repository? e.g. your version is a branch?

@TFenby

I haven't made any public changes to my CouchPotato fork. I haven't updated Sick Beard from master because I was hoping this pull request could just be merged in, so it's a few commits behind. If this patch creates any bugs, you can comment here or send me a message.

@TFenby

So... is there something holding this back from being pulled? I'll gladly make any necessary changes.

@d248066

could someone please pull this into the main release. this code is working great and very helpful.

@thezoggy

you do realize that there is 69 pull requests. some have been around for a year. it takes awhile sometimes to test things out. and anything that's a major change for sb needs to be reviewed. why smaller changes get implemented faster. hell.. we've been working on the custom_naming branch on and off for a few months now.. just been busy. dont take anything personally.

@TFenby

Of course, I just want to make sure there's nothing I missed or need to change. Should I start merging your master into mine to keep this up to date? This is actually my first pull request, so forgive my noob-ness!

@simonk83

I may switch to your branch for the time being schitso to help test this out. Considering this is by far the most starred issue you'd hope it'll get some prioritisation, but I guess there's plenty of other stuff to add. The life of a coder is never quiet :)

One thing I was considering the other day is that occasionally (more often recently), downloads from NZBmatrix will fail immediately when passed to SAB due to "Empty NZB" or some such error. In cases such as these, you probably don't want your patch to try a new version, but rather re-attempt the same version again later as it usually sorts itself out eventually. Even better, perhaps, would be for it to try a new provider altogether when this error happens (I find that the built in Sickbeard newznab type provider tends to have the file available when NZBM chucks out this error).

Just food for thought while you wait for this to be pulled anyway. Appreciate the patch, great work :)

@thezoggy

in 0.7.x of sab we added the feature of retrying nzb if it failed... thus people can just do that as a band-aid fix for nzbmatrix

@simonk83

Good stuff. I'm still on 0.6.15 so I'll check that out.

@julestop

Hi,
Big noob over here.
I have been using SickBeard, Sabnzbd, and CouchPotato for the past couple months now and love it all.

I was wondering if there is a way of getting this patch on windows?
I installed ubuntu on a partition but had a lot of trouble mapping a network drive so that sickbeard could postprocess in my Boxee Box's network HDD. So now I'm back in Windows and this is the only issue I've had with Sickbeard and would love to have the patch.
So if anyone has any recommendations on mapping a network drive in ubuntu (like it is in windows, showing up as a permanent HDD with a proper folder path that doesn't start with 'smb' which sickbeard doesn't recognize) that would be awesome, otherwise, I would love a way of patching my windows version.

Sorry for the long post :P

@julestop

Never mind guys, I got Ubuntu working. I was able to use fstab to map the drives permanently.
Now I just have to boost the wireless speeds. I have a 8-10MB/s throughput with Windows and a 2.2MB/s throughput with Ubuntu... more tinkering, here I come.

@simonk83

Ok, switched to your branch schiso. I'll let you know if anything funky happens :)

@TFenby

Thanks, the more testers the better! I'll work on getting this up to date with the main branch this weekend.

Tyler Fenby added some commits Mar 30, 2012
Tyler Fenby Merge git://github.com/midgetspy/Sick-Beard into merge
Conflicts:
	sickbeard/processTV.py

Merge midgetspy/Sick-Beard/master
25aaacb
Tyler Fenby unix2dox on processTV.py
Make vim stop showing me escape codes.
b361c12
@julestop

Hey guys, although I got Linux running, the OS is unstable on my hardware.
Do you guys know when this patch will be available on a windows build of sickbeard?

@midgetspy midgetspy and 1 other commented on an outdated diff Apr 13, 2012
sickbeard/processTV.py
def logHelper (logMessage, logLevel=logger.MESSAGE):
logger.log(logMessage, logLevel)
return logMessage + u"\n"
-def processDir (dirName, nzbName=None, recurse=False):
+def setWanted(show, season, episode):
+ returnStr = ''
+ if show == None or season == None or episode == None:
+ errMsg = "Programming error: invalid setWanted paramaters"
+ ui.notifications.error('Error', errMsg)
+ returnStr += logHelper(json.dumps({'result': 'error'}), logger.ERROR)
+
+ showObj = sickbeard.helpers.findCertainShow(sickbeard.showList, show)
+
+ if showObj == None:
+ errMsg = "Error", "Show not in show list"
+ ui.notifications.error('Error', errMsg)
+ returnStr += logHelper(u"Error: show not in show list", logger.ERROR)
+ returnStr += logHelper(json.dumps({'result': 'error'}), logger.ERROR)
@midgetspy
midgetspy Apr 13, 2012

What's the purpose of this json?

@TFenby
TFenby Apr 13, 2012

I just copied it over from the set status function in webserve.py (the json.dumps)

@midgetspy
Owner

Can SAB be set to delete failed and still send the post processing request? Deleting it doesn't seem like SB's job if we can help it. Other than that I like the looks of this a lot, I will look to pull it soon. Thanks.

@thezoggy

this should be put on hold, there is a bunch of stuff we need to address in sab.py before going forward with this.. also can probably help make this go alot smoother. more info later

Tyler Fenby added some commits Apr 14, 2012
Tyler Fenby Merge remote-tracking branch 'ms/master' 7c3699e
Tyler Fenby Clean up setWanted
Remove JSON. Fix some logging things.
d047aec
Tyler Fenby Merge remote-tracking branch 'ms/master' d894bd5
@TFenby

I see there's a development branch now. Should I re-open this pull request on that?

@spo0nless

I have been wanting this feature in SB because It recently downloaded a fake (passworded) release and I was hoping for a way for SB to ignore that one and wait for a different one. Thanks for this.

Tyler Fenby Merge remote-tracking branch 'ms/master'
Conflicts:
	sickbeard/postProcessor.py

Also ran unix2dos on postProcessor.py
a103dcc
@TFenby

Yea/nay? Anything else I can do?

@TFenby

You're killin' me here.

@thezoggy

sorry, been out of the country for a few weeks (korea/china). still playing catchup.. midget appears to be on holiday as well..

@TFenby

Ah! Fun stuff. Whenever you get the chance. :)

@TFenby

So, I'm back up to date on the master branch. Anything I can do to get this pushed through? :P

@midgetspy
Owner

About your previous question, yes please submit against development. I will review this and look to pull it soon hopefully. It looks like this adds all the framework for a user to manually mark a release as no good and have SB find a different one, right?

@TFenby

Will do. Do you want me to squash the commits?
Currently, it functions like this:
Disable "Post-Process Only Verified Jobs" in SABnzbd.
SABnzbd tells us when a download fails--no need for user intervention if we sent it the nzb.
On the manual post processing page, there's a checkbox to mark a manually downloaded nzb as bad.

I haven't tested it with any other nzb downloaders, and there's no way for a user to mark an auto-selected nzb as bad, since there shouldn't be a need to (with SABnzbd, at least).

I can make any changes you want to make this work how you think it should.

@mano3m

An alternative would be to ask sabNZBd if the download failed instead of postprocessing failed downloads, and have sabnzbd delete the failed download (without the need to postprocess):

Below a modified snippet from code that I posted to couchpotato:

def checksnatchedfailed(self, nzbname)
    params = {
        'apikey': APIKEY,
        'mode': 'history',
        'output': 'json'
    }
    url = SABHOST + "api?" + tryUrlencode(params)

    try:
        history = json.load(urllib2.urlopen(url))
    except:
        return False 

    # Go through history items
    for slot in history['history']['slots']:
       if slot['name'] == nzbname and slot['status'] == 'Failed':

            # Delete failed download
            params = {
                'apikey': APIKEY,
                'mode': 'history',
                'name': 'delete',
                'del_files': '1',
                'value': slot['nzo_id']
            }
            url = SABHOST + "api?" + tryUrlencode(params)
            try:
                data = self.urlopen(url, timeout = 60, show_error = False)
            except:

            # Return failed
            return True

    return False
@TFenby

Oooh. I like that more. Does NZBGet have such an option in its API? I don't see it explicitly mentioned on their reference. Like I said, as it is this patch only works with SABnzbd, and it'd be nice if that weren't the case.

@thezoggy

in sb we use the old text based api.. we dont have the nbo. thus your idea really wont work just yet. also we have a 'sabpoller' that eliminates the whole need for the sabToSickbeard script at all..

@mano3m

Dont know what you mean with nbo? The above code works with import json, urllib2 both standard in python. Not much more is required. If we would poll sab instead of use the sabtosickbeard my code snippet together with this pull request would be perfect :)

@mano3m

Let me know if you want me (us?) to merge this pull request, with my little addition into the sabpoller pull request #278

@TFenby

It would be only a few minor changes to get both working together.

@TFenby TFenby referenced this pull request Jul 31, 2012
Closed

Failed Download Handling #445

@TFenby TFenby closed this Jul 31, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment