report enum and get_reportid #74
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #74 +/- ##
===========================================
+ Coverage 74.77% 76.38% +1.61%
===========================================
Files 18 18
Lines 880 940 +60
Branches 93 93
===========================================
+ Hits 658 718 +60
Misses 215 215
Partials 7 7
Continue to review full report at Codecov.
|
|
beforehand i used a wrapper to get new reports. The report enum is not tested and not complete. I think it's a good place to put some useful, easy accessible information about reports. |
|
to-do: remove tuples. |
|
How long is a good wait time for the report to finish? |
There was a problem hiding this comment.
Good work entering the ReportType enumerations, but get_reportid may be too complex for current scope. Recommend moving that to a separate PR for separate scrutiny, or leaving out entirely / leave up to the user to make their own wrapper.
Also see notes on enum and enum34. Us not getting errors is actually a fluke of the test suite's dependencies (which indicates a need to rearrange the test steps).
mws/apis/reports.py
Outdated
| assert request.parsed.ReportRequestInfo.ReportProcessingStatus == '_SUBMITTED_' | ||
| request_id = request.parsed.ReportRequestInfo.ReportRequestId | ||
|
|
||
| sleep(120) |
There was a problem hiding this comment.
I don't like the idea of adding sleep timers into the code base for anything other than explicit throttle handling. Further, the naming of this method (which ought to be get_report_id, but that's a nitpick) implies it simply parses the response to return the report ID, not that it makes new requests.
The methodology comes from a good place, but I think it'd be better to think about breaking this off into a multi-thread or multi-process operation of some kind, using a pattern of a "thread with return value". This would prevent locking a user's main thread for up to 3 minutes at a time, allowing them to keep running their own operations.
Even that may be too complex for this project's scope. I recommend removing or forking this method, and focusing this PR on the ReportTypes enumeration only.
There was a problem hiding this comment.
thanks deleted it for now. I will look into those things. Probably threading?
mws/apis/reports.py
Outdated
| # pass | ||
|
|
||
|
|
||
| class AllReports(Enum): |
There was a problem hiding this comment.
Class should be ReportType, in keeping with the expected naming from Amazon MWS.
| """ | ||
| from __future__ import absolute_import | ||
| from time import sleep | ||
| from enum import Enum |
There was a problem hiding this comment.
enum is not available in Python2.7 (which we still support).
It just so happens that we get no errors on this because flake8 (code style checker) uses enum34 as a dependency, which is a backport for Py3.4's enum.
Thus, enum34 would need to be added as an extra requirement for Python versions below 3.4, when enum was introduced. See the setup.py for flake8 for an example of this (how to add a requirement based on python version).
There was a problem hiding this comment.
It seems with pip it got backported. That's why the tests work.
So long we keep it very basic it should work, i did test the basics:
python2.7 Python 2.7.15rc1 (default, Apr 15 2018, 21:51:34) [GCC 7.3.0] on linux2 Type "help", "copyright", "credits" or "license" for more information. >>> from enum import Enum >>> class ReportType(Enum): ... IN = 'something' ... >>> type(ReportType) <class 'enum.EnumMeta'> >>> ReportType.IN <ReportType.IN: 'something'> >>> ReportType.IN.value 'something'
There was a problem hiding this comment.
also you are right. It just appears that the import falls back to enum34 without doing anything.
Like one guy points out:
from enum import Enum # for enum34, or the stdlib version
also in the official docs for enum34.
So we are lucky for once :)
There was a problem hiding this comment.
Our requirements file still lists flake8, and Travis CI is building requirements off that, so enum34 becomes available as a result. Slight organizational problem there (whoops!), but not a big deal.
We'll still need a python<3.4 requirement for enum34 in the setup in order to allow for pip deployment. Otherwise, it will break in production environments.
and lets see if the test now fails
A current PR is using `enum`, which was added in Py3.4. That should have produced an error for a missing module in Py2.7 testing, but `flake8` loads `enum34` as a dependency into the environment first. This contaminates the test suite prior to running tests, hiding what should be an error.
|
FYI, some post-merge changes committed on setup.py to streamline Py version installs. Future changes: we'll test out the ReportType enumerations and try to tie them into request methods more closely. Also need documentation to point them out and how to make use of them. |
* Release for 0.8.0 (python-amazon-mws#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. (python-amazon-mws#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. * BUG: GetReportList missing from next token ops Added in with a hotfix. * Version push * version push * version bump (corrects wrong version from before) * Bugfix inbound constructor (0.8.3 release) (python-amazon-mws#57) * version bump * No from_address assignment needed in Inbound init Similar fix as commit on `develop`, which will come in with 1.0 release. Also, version bump for new bugfix release. * Remove import for ExpatError We do not support Python < 2.7, in which ParseError is available; import for ExpatError is not necessary. * Comments added for later work. * Added '.' at end of some params to test Params ought to pass through `enumerate_keyed_param` whether they end with '.' or not. * Added test for dict_keyed_params * Added test for enumerate_keyed_param Also fix the existing one (wrong info used) * Convert test methods to testcase class * Create example_response.txt File contains an example response from MWS, taken directly from MWS documentation. To be used for testing utilities. * Change test methods to test case classes * Add `download_url` to setup.py * Fix issue python-amazon-mws#60 (python-amazon-mws#61) * Fix issue#1 Always return an interable list. Stolen from https://github.com/bloodywing Thanks! * Fix issue python-amazon-mws#60 We must sort the params before encoding and concatenating or we will fail our calls. This is a nice hybrid between the old version and a nicer more pythonic way of doing things with .join and not just appending to strings and then hacking the last ampersand off. * Sorting of dict keys instead of the dict object Essentially the same output, slightly more explicit for an easier time reading it later. * Feeds example. (python-amazon-mws#64) * Feeds example. Use of ".parsed" changed. (python-amazon-mws#66) * Feeds example. * Feeds example. Use of ".parsed" changed. * Subscriptions api (python-amazon-mws#67) * Fix issue#1 Always return an interable list. Stolen from https://github.com/bloodywing Thanks! * Fix issue python-amazon-mws#60 We must sort the params before encoding and concatenating or we will fail our calls. This is a nice hybrid between the old version and a nicer more pythonic way of doing things with .join and not just appending to strings and then hacking the last ampersand off. * Sorting of dict keys instead of the dict object Essentially the same output, slightly more explicit for an easier time reading it later. * Start working on the subscriptions API * More updates to subscription to test. * missed off Subscription * remove subscription, example appears to not require this? * subscription required for subscription calls, not on destination, * flake8 make creation of call consistent accross this file. * indentation error. * Subscriptions (#9) * Fix delete subscription * Fix delete subscription * Subscriptions (#10) * Fix delete subscription * Fix delete subscription * _type -> notification_type * small doc update * Subscriptions (#11) * Fix delete subscription * Fix delete subscription * _type -> notification_type * small doc update * change subscriptions attributes_list to be attributes (dictionary) then map the key and value into the correct style for enumeration * Subscriptions (#12) * Fix delete subscription * Fix delete subscription * _type -> notification_type * small doc update * change subscriptions attributes_list to be attributes (dictionary) then map the key and value into the correct style for enumeration * flake8 after merge issues * Subscriptions (#13) * Fix delete subscription * Fix delete subscription * _type -> notification_type * small doc update * change subscriptions attributes_list to be attributes (dictionary) then map the key and value into the correct style for enumeration * flake8 after merge issues * missed off subscriptions in updated params * flake8 * fix get_subscription * move delivery_channel to the end of the params so it can be a default * clean method (python-amazon-mws#65) * clean input parameter * Feeds example. Use of ".parsed" changed. (python-amazon-mws#66) * Feeds example. * Feeds example. Use of ".parsed" changed. * see conversation pull request * modified tests to fit new clean method * revert start in subapi * git ignore * d * r * travis bugfix * call clean method for params too * reuse modifiers for request params also it's helpful to understand how we modify params * param test for wrong datatype exception * clean all parameters at once see idea from GriceTurrble in this pull request * remove dt_iso_or_none no need for this * Feeds example. Use of ".parsed" changed. (python-amazon-mws#66) * Feeds example. * Feeds example. Use of ".parsed" changed. * Revert "Feeds example. Use of ".parsed" changed. (python-amazon-mws#66)" This reverts commit ba0f363. * Revert "Feeds example. Use of ".parsed" changed. (python-amazon-mws#66)" This reverts commit e87012b. * Revert "Feeds example. Use of ".parsed" changed. (python-amazon-mws#66)" This reverts commit 77d3ea7. * Revert "Feeds example. (python-amazon-mws#64)" This reverts commit 2a18583. * Added support for zip files. * ignore files * Zip files downloaded to current directory. * Added unzip() function in DataWrapper object. * Minor updates, correcting my own mistakes from recent merge * Remove "assert_no_token". Not used except in a single test. * Fallback to _response_dict if _rootkey is not present in response dict (python-amazon-mws#72) This is the case while fetching some of the reports. I particularly found this in '_GET_XML_RETURNS_DATA_BY_RETURN_DATE_'. In this case the data is returned but not in rootkey. This will at least let the user parse the data on there own * Move flake8 checking after pytest calls A current PR is using `enum`, which was added in Py3.4. That should have produced an error for a missing module in Py2.7 testing, but `flake8` loads `enum34` as a dependency into the environment first. This contaminates the test suite prior to running tests, hiding what should be an error. * report enum and get_reportid (python-amazon-mws#74) * report enum and get_reportid * removed tuples since we do now handle the decoding better * removed get_request id and lets see if the test now fails * Move flake8 checking after pytest calls A current PR is using `enum`, which was added in Py3.4. That should have produced an error for a missing module in Py2.7 testing, but `flake8` loads `enum34` as a dependency into the environment first. This contaminates the test suite prior to running tests, hiding what should be an error. * extra require python2.7 enum34 * Streamline package requirements for enum34 * Bugfix, my mistake. Import setuptools needed * ... and sys * Push dev version num We've gone several versions up, technically, without changing the version num to match. May as well start somewhere. * Dev version num push (match setup) * Call to requests.request needs explicit timeout (python-amazon-mws#87) * Call to requests.request needs explicit timeout Call to requests.request doesn't explicitly mention timeout, without which could lead to zombie processes as default timeout is infinity (None). * Add marketplace ID enums #19 (python-amazon-mws#84) * enum marketplaces * typos * removed domain parameter, pls use region now, see MARKETPLACES * very basic tests * python-amazon-mws#91 add BR and AU and sort alphabetical * single quotes * better name for variable * idiomatic names, thanks to jameshiew * Changed excludeme default to be xsd1.1 compatible (python-amazon-mws#97) Occasionally, the MWS API for Products will throw an error, citing a non-xsd1.1 boolean as an example. Making the default value for the keyword argument excludeme into "false" fixes this. * Revert "Merge branch 'develop' into develop" This reverts commit 1a15914, reversing changes made to a7886c7.
| class ReportType(Enum): | ||
| """Better names for reports.""" | ||
|
|
||
| # Listing Reports |
There was a problem hiding this comment.
ALL_LISTINGS = 'GET_MERCHANT_LISTINGS_ALL_DATA'
Solving #21
and providing function for parsing request report id and getting a reportid.
#75 decodes now by default to hopefully the correct string. So this get's much easier
example of new functionality. With #75 implemented.