Requests via proxy and almost of InboundShipments requests#22
Requests via proxy and almost of InboundShipments requests#22GriceTurrble merged 13 commits intopython-amazon-mws:developfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #22 +/- ##
===========================================
- Coverage 48.85% 46.31% -2.54%
===========================================
Files 4 4
Lines 393 475 +82
Branches 22 27 +5
===========================================
+ Hits 192 220 +28
- Misses 192 245 +53
- Partials 9 10 +1
Continue to review full report at Codecov.
|
|
Can you please tell me what to do for passing codecov? Because I really don't know how to make tests for this |
|
Thanks for the PR! Don't worry about the code coverage, at the moment it is just for some parts of that have already had bugs with regards to py2/3 compatibility. To make tests for other parts of the code will be big work. |
|
Ok, thanx for reply:) |
|
So what will be with this pull request? Will it be merged? |
|
@Eyeless95 Yes definitely, will review it in the next week |
mws/mws.py
Outdated
|
|
||
| def __init__(self, access_key, secret_key, account_id, region='US', domain='', uri="", version="", auth_token=""): | ||
| def __init__(self, access_key, secret_key, account_id, region='US', domain='', uri="", version="", auth_token="", | ||
| proxies=None): |
There was a problem hiding this comment.
I'm guessing proxies is meant to be a protocolless URL like www.example.com ?
There was a problem hiding this comment.
If it is just the one URL, could we call this proxy and self.proxy instead of proxies ?
There was a problem hiding this comment.
No, protocol is required, and I made http & https to avoid troubles with type of proxy, now this proxy settings works on my project, so this feature is tested for work
mws/mws.py
Outdated
| parsed_response.response = response | ||
| return parsed_response | ||
|
|
||
| def set_proxies(self): |
There was a problem hiding this comment.
I think it would make sense to call this get_proxies or something similar, as we are not setting anything here on the instance, rather just making the dictionary and returning it
There was a problem hiding this comment.
Obviously I made this method because request didn't pass codecov :)
There was a problem hiding this comment.
Yes I'll add a note to the contribution docs saying not to worry about codecov (for now)
|
@Eyeless95 am I good to make the minor naming changes mentioned? You said you're using it already, renaming |
|
am I good to make the minor naming changes mentioned? - Sure, that's ok :) |
|
Hey @Eyeless95, so over a month ago I said I would review the PR, how time flies 😄 ! I've set aside a day next week (Wednesday) to get everything for |
|
It is not a problem, I added few corrections to name of methods at this time, thanx for replying me:) |
|
Hey @Eyeless95, I made some small changes to the PR to do with the proxies functionality. The proxy stuff is good to merge in, if you are able to split it out into a separate PR that would be great, but otherwise no worries, I still need to test the inbound shipments a bit more then it can all be merged in at once. Though I am not sure I will be able to test all of the methods as the MWS account I am playing with does not use this functionality so much. One thing you can maybe help with: I tried to use this method and got the following stack trace. >>> InboundShipments.get_inbound_guidance_for_sku(sku_list=('my_sku', ))
Traceback (most recent call last):
File "<input>", line 1, in <module>
InboundShipments.get_inbound_guidance_for_sku(('my_sku', ))
File "/Users/james/Desktop/mws/mws/mws.py", line 639, in get_inbound_guidance_for_sku
return self.make_request(extra_data=data)
File "/Users/james/Desktop/mws/mws/mws.py", line 200, in make_request
request_description = '&'.join(['%s=%s' % (k, quote(params[k], safe='-_.~')) for k in sorted(params)])
File "/Users/james/Desktop/mws/mws/mws.py", line 200, in <listcomp>
request_description = '&'.join(['%s=%s' % (k, quote(params[k], safe='-_.~')) for k in sorted(params)])
File "/Users/james/.pyenv/versions/3.6.1/lib/python3.6/urllib/parse.py", line 775, in quote
return quote_from_bytes(string, safe)
File "/Users/james/.pyenv/versions/3.6.1/lib/python3.6/urllib/parse.py", line 800, in quote_from_bytes
raise TypeError("quote_from_bytes() expected bytes")
TypeError: quote_from_bytes() expected bytes
|
|
Hi folks, I have some changes to the InboundShipments class that I built in my own fork of the original package (by czpython), including some calls that are missing in this PR. See here: https://github.com/GriceTurrble/python-amazon-mws/blob/master/mws/mws.py#L797 Note that my package is Python3 compatible, but not very backwards compatible to Py2. I'd like to contribute changes such as the Edit: just for clarity, I do not currently have an MWS account to play with, but I did previously work at a firm in which the main focus was creating and editing FBA shipments. Those methods are battle-tested, I assure you. :) |
|
@jameshiew In response to your stack trace, the |
|
Hey @GriceTurrble , I haven't been able to look at the repo lately as much I've wanted to, but feel free to make a PR or further help with this one, especially if your methods are battle tested! Thanks for the info about the stack trace, perhaps the enumerate_params is something the library should do. It may be some of the work can split out from this PR and integrated sooner (e.g. the proxy functionality @Eyeless95 has done), and then some other InboundShipments work from this PR and from your repo can be integrated later. |
|
Been a while, but I'm settled in the new job now and have some bandwidth to work on this. Will have a new PR specific to the InboundShipments section shortly. That will create some conflict with the work @Eyeless95 contributed already, and for that I apologize, but I feel that what I have is a bit more complete. I'll continue on a separate PR when I'm ready to bring it in. Thanks. |
GriceTurrble
left a comment
GriceTurrble
left a comment
There was a problem hiding this comment.
#33 accepted recently with some of the same changes to FulfillmentInboundShipments. However, this PR covers some methods that 33 does not, and I'd like to merge them in.
Please commit the following changes so we can proceed:
- Update based on current state of the develop branch, taking changes to InboundShipments and Finances into effect.
- Remove method
InboundShipments.get_service_status. This is overwriting the baseMWS.get_service_statusunnecessarily. - The following methods are duplicate, and should be removed from this PR:
- estimate_transport_request
- get_bill_of_lading
- get_package_labels
- get_prep_instructions_for_asin
- get_prep_instructions_for_sku
- get_transport_content
- list_inbound_shipment_items
- list_inbound_shipment_items_by_next_token (covered by next_token_action decorator on list_inbound_shipment_items)
- list_inbound_shipments
- list_inbound_shipments_by_next_token (covered by next_token_action decorator on list_inbound_shipments)
- void_transport_request
- The following methods are new, and should be included in this PR (leave in place):
- confirm_preorder
- confirm_transport_request
- get_inbound_guidance_for_asin
- get_inbound_guidance_for_sku
- get_pallet_labels
- get_preorder_info
- get_unique_package_labels
- put_transport_content
- Remove the change to version number in setup.py (we'll push that version up on our side shortly).
- Remove
test_get_service_status_inbound(redundant coverage: see point 2 above)
Codecov Report
@@ Coverage Diff @@
## develop #22 +/- ##
==========================================
- Coverage 45.57% 44.88% -0.7%
==========================================
Files 4 4
Lines 667 704 +37
Branches 66 70 +4
==========================================
+ Hits 304 316 +12
- Misses 348 374 +26
+ Partials 15 14 -1
Continue to review full report at Codecov.
|
GriceTurrble
dismissed
their stale review
Committed fixes that match my review notes.
* Merge recent changes in master to develop branch (#45) (#47) * Release for 0.8.0 (#42) * Added Finances API Feature * .setup.py: bump version to 0.7.5-dev0 * Split out request_description building from make_request() So that it can be tested more easily, and refactored * Split out building the initial params dict from make_request() So that it can be tested more easily * Add fake MWS credentials pytest fixture * test_service_status() should use the pytest fake credentials fixture * Add more pytest fixtures (access_key, secret_key, account_id, timestamp) * Add test for calc_request_description() * Split out calc_request_description() into more statements So that it is easier to debug * Fix calc_request_description - don't include leading ampersand * Don't do automated deployments via Travis (for the moment) * Update README.md badges * InboundShipments, next_token_action decorator, and some style cleanups. (#33) * Testing out git commits from VS Code * Reverting the test commit * Adding VS Code settings to gitignore. * Style fixes * MWS.enumerate_param deprecated: now using utils.enumerate_param and utils.enumerate_params * InboundShipments fleshed out; added `utils.next_token_action` decorator; deprecated separate methods for `...by_next_token()` * Bugfix, rename `_enumerate_param` to `enumerate_param` (no need for private) * Fix for next_token issues. * TravisCI flake8 complaining, fixes. * Minor flake8 complaint. * Hack to get flake8 to stop complaining. * Strip pylint disables to clear line length issue. * Correction to keyed params, now tests every item in values sequence to ensure all are dicts. * Add tests for param methods in utils. * Add test for next token decorator. * Adding 'InboundShipments' to `__all__` * Assigning response a default in __init__ for DictWrapper and DataWrapper * Unneeded line breaks removed + docstring formatting * Comment corrected. They're tuples, not sets. * Finances methods updated to use next_token_action decorator * Create .travis.yaml * Update .gitignore * Removing deploy code from local travis * Delete .travis.yaml * Pushed to 0.8.0-dev0 Recently added functionality to InboundShipments, as well as Finances API. These constitute feature additions with backwards compatibility, which calls for a minor version update. * Adding Python 3.6 category We are testing in 3.6 in Travis anyway. May as well include the note. * Specified master and develop branches for badges Ensured the badges for Travis and Codecov are pointing to the appropriate branches (used to be pointing to default, which was master in both cases). * Updated comments throughout module No substantial code changes, comment changes only. Also ensured all docstrings follow same format. * Fixed docstring formatting Also made slight update to docstring for ObjectDict to more clearly note what it does, vs what the original code did. * Fix for flake8 (trailing whitespace) * Fix for flake8 (trailing whitespace) * Bump to 0.8.0 (drop dev tag) for release * Bug: Incorrect use of `super` for back-compat Using the old-style `super` syntax to comply with Python 2.7 compatibility. Not revealed in tests, because current tests don't touch the APIs. Whoops! * Added back old object names in case needed Old names `object_dict` and `xml2dict` added back in case the old objects are being used directly by some users. To be removed in 1.0.0 release down the road. * Version bump for agent string. * Hi, howyadoin? * Format request url with str.format Remove old-style string formatting for sake of clarity. * Refactor timestamp method. * Naming cleanups * Single-source package version for use in agent string. * MWS refactored, split into different modules. * Version bump for feature * Move and incorporate new MerchantFulfillment API * merge artifacts removed [broken] * Methods from PR #22 added. * Changes from PR #53 merged
4 participants
No description provided.