Skip to content

Py3: Fix error handling in download plugin#1081

Merged
cvium merged 3 commits intoFlexget:developfrom
jeroenl:save_error_page_py3
Sep 9, 2016
Merged

Py3: Fix error handling in download plugin#1081
cvium merged 3 commits intoFlexget:developfrom
jeroenl:save_error_page_py3

Conversation

@jeroenl
Copy link
Copy Markdown
Contributor

@jeroenl jeroenl commented Apr 24, 2016

While trying to store an error page, the plugin returned:
TypeError: write() argument must be str, not bytes

While trying to store an error page, the plugin returned:
TypeError: write() argument must be str, not bytes
@landscape-bot
Copy link
Copy Markdown

Code Health
Repository health decreased by 0.01% when pulling 74eeff0 on jeroenl:save_error_page_py3 into ff00891 on Flexget:develop.

if not os.path.isdir(received):
os.makedirs(received)
filename = os.path.join(received, '%s.error' % entry['title'].encode(sys.getfilesystemencoding(), 'replace'))
filename = os.path.join(received, '%s.error' % entry['title'].encode(sys.getfilesystemencoding(), 'replace').decode(sys.getfilesystemencoding()))
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.

Hmm, we are encoding then decoding?? that does not sound right?

what error did you get?

Copy link
Copy Markdown
Contributor Author

@jeroenl jeroenl Apr 25, 2016

Choose a reason for hiding this comment

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

The error came below. For this line, I noticed that the filename shows up as:
path/to/b'entry title'.error
as opposed to (in Py2)
path/to/entry title.error

Encode then decode in this case would at least drop characters that are not valid in the file system encoding, which I'm guessing at some point fixed a bug. I do wonder though if maybe we shouldn't change the way the file name is generated. Perhaps using pathscrub?

@stevezau
Copy link
Copy Markdown
Contributor

hmm yeah i think we can just do entry['title'] now as we enforce it to be text here

Flexget/flexget/entry.py

Lines 189 to 194 in 646d6ed

if isinstance(value, oldstr):
# Allow Python 2's implicit string decoding, but fail now instead of when entry fields are used.
# If encoding is anything but ascii, it should be decoded it to text before setting an entry field
try:
value = value.decode('ascii')
except UnicodeDecodeError:

do you want to update the PR?

@gazpachoking
Copy link
Copy Markdown
Member

Maybe we should use path.py for these filename operations rather than raw text

@jeroenl
Copy link
Copy Markdown
Contributor Author

jeroenl commented Apr 26, 2016

How's this?

@landscape-bot
Copy link
Copy Markdown

Code Health
Code quality remained the same when pulling 3ec833b on jeroenl:save_error_page_py3 into ff00891 on Flexget:develop.

@stevezau
Copy link
Copy Markdown
Contributor

hmm looks ok to me.. @gazpachoking ?

@gazpachoking
Copy link
Copy Markdown
Member

Looks fine to me.

@cvium
Copy link
Copy Markdown
Contributor

cvium commented Aug 28, 2016

@jeroenl Wanna sync this so we can merge?

@cvium cvium merged commit 5581ce3 into Flexget:develop Sep 9, 2016
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.

5 participants