Skip to content
This repository was archived by the owner on Jan 4, 2025. It is now read-only.

Ensure correct decoding of bytes#168

Merged
GriceTurrble merged 1 commit intopython-amazon-mws:developfrom
Amedeo91:fix_decoding_issue
Jun 25, 2020
Merged

Ensure correct decoding of bytes#168
GriceTurrble merged 1 commit intopython-amazon-mws:developfrom
Amedeo91:fix_decoding_issue

Conversation

@Amedeo91
Copy link
Copy Markdown
Contributor

@Amedeo91 Amedeo91 commented Mar 22, 2020

No description provided.

@Amedeo91 Amedeo91 mentioned this pull request Mar 28, 2020
Closed
@Amedeo91
Copy link
Copy Markdown
Contributor Author

Amedeo91 commented Mar 29, 2020

Hello, I need this fix released soon.
Could you please give me a release date?
Thanks,
Amedeo

@elcolumbio
Copy link
Copy Markdown
Contributor

elcolumbio commented May 11, 2020

Hello @Amedeo91,
Thank you for your PR.
I was doing some work on the encoding and decoding part in the past.
You could have a look at this branch and the DataWrapper.
Especially this part:

python-amazon-mws/mws/mws.py

Lines 159 to 166 in d0d3e40

# a better guess for the correct encoding
self.original.encoding = self.guess_encoding()
textdata = self.original.text
# We don't trust the amazon content marker.
try: # try to parse as xml
self._xml2dict(textdata)
except ExpatError: # if it's not xml its a plain textfile, like a csv
self.textdata = textdata

If you want to make a hotfix for your app, i have two tips for you.

  • never depend on encoding defaults, always be explicit. Before python 3.6 they were different on windows and everyone can still set their own.
  • the response from amazon isn't always utf-8. especially here i used chardet to find out which encoding the text has, see in my link above.

GriceTurrble
GriceTurrble reviewed Jun 25, 2020
View reviewed changes
GriceTurrble
GriceTurrble approved these changes Jun 25, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@GriceTurrble GriceTurrble left a comment

Choose a reason for hiding this comment

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

Small fix, seems like a sizable impact and isolated from most other functionality we've been using.

The unit test included is appreciated, though I fear it may turn up strange results, as noted in the unrelated comment. Still, doesn't seem to be hurting anything from here.

Will adjust later if a problem arises. For now, accepting and pushing the change in.

Thank you!

@GriceTurrble GriceTurrble merged commit dc0fc1e into python-amazon-mws:develop Jun 25, 2020
Lacrymology added a commit to Shiphero/python-amazon-mws that referenced this pull request Jul 10, 2020
@Lacrymology
Merge remote-tracking branch 'upstream/develop' into develop
66ad31f 
* upstream/develop:
  Add Feeds utility to convert dict FeedOptions to string (python-amazon-mws#181)
  Ensure correct decoding of bytes (python-amazon-mws#168)
  Compatibility fix (python-amazon-mws#124)
  Implement a couple of function for the OutboundShipments API. (python-amazon-mws#96)
  Added OutboundShipments.create_fulfillment_order (python-amazon-mws#95)
  added UpdateReportAcknowledgements (python-amazon-mws#101)
  Update feeds.py (python-amazon-mws#166)
  added easyship api support for indian marketplace (python-amazon-mws#169)
  Moving Slack link up, removing gitter chat link
  add all current marketplaces and alphabetize by country code (python-amazon-mws#155)
  Updated slack invite link
  Slack invite link (python-amazon-mws#152)
  Slack invite link
  fix clean_params (python-amazon-mws#106)
  add in NL marketplace information
  Fix flake8 warnings
  Update develop branch CI + package metadata
  include mws.apis in setup.py
  Fix bug: Remove trailing comma
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@GriceTurrble GriceTurrble GriceTurrble approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Amedeo91 @elcolumbio @GriceTurrble