Throttling and SDK updates#19
Conversation
YPCrumble
left a comment
There was a problem hiding this comment.
@sergioteula thanks for this! Just added a review. Would be really nice to start the unit test suite as well - that would help for writing regression tests as we fix bugs.
| else: | ||
| self.throttling = float(throttling) | ||
| if not self.throttling > 0: | ||
| raise ValueError |
There was a problem hiding this comment.
@sergioteula might be nice to include a message telling the user exactly what the issue is vs. having them need to look at the code to understand why the ValueError occurred. Something like "Throttling cannot be less than or equal to zero". Same thing for the ValueError above.
There was a problem hiding this comment.
When an error is raised, the try clause handles the error and throws below error message. I made it like this to avoid repeating the error 3 times.
python-amazon-paapi/amazon/paapi.py
Line 50 in 048e4b3
sergioteula
left a comment
There was a problem hiding this comment.
I will update pull request with some changes. Thanks!
| else: | ||
| self.throttling = float(throttling) | ||
| if not self.throttling > 0: | ||
| raise ValueError |
There was a problem hiding this comment.
When an error is raised, the try clause handles the error and throws below error message. I made it like this to avoid repeating the error 3 times.
python-amazon-paapi/amazon/paapi.py
Line 50 in 048e4b3
Fix TypeError in underlying API when self.pool is None for async request
YPCrumble
left a comment
There was a problem hiding this comment.
Looks great - thank you @sergioteula !
I have updated throttling to allow values greater than 1 and also False to disable it. On the other hand, I have removed amightygirl.paapi5-python-sdk as a dependency and added the official SDK directly in the project to allow fixing bugs.