Tap Shopify Performance Enhancements #34
Conversation
…on to 1.1.9. • Added async capability for orders and upgraded version to 1.2.0.
…her than yielding results. Removed async for abandoned checkouts as you cannot paginate for this.
|
Hi @siddharthsudheer, thanks for your contribution! In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. |
|
You did it @siddharthsudheer! Thank you for signing the Singer Contribution License Agreement. |
| async with session.get(url=url, headers=headers, params=params) as response: | ||
| if response.status == 200: | ||
| return await response.json() | ||
| elif response.status == 429: |
There was a problem hiding this comment.
There appears to be a bug here with status codes != 200, 429, and 500-599. Getting a 401, for example will miss.
| return ranges | ||
|
|
||
| async def _get_async(self, url, headers=None, params=None, retry_attempt=0): | ||
| headers = {**headers, "Connection": "close"} if headers else {"Connection": "close"} |
There was a problem hiding this comment.
There is some missing code to connect this with the auth info. ie its missing the header: headers['X-Shopify-Access-Token']
| @@ -1,6 +1,7 @@ | |||
| import math | |||
| import functools | |||
| import datetime | |||
| from datetime import datetime, timedelta | |||
There was a problem hiding this comment.
Changing this import broke code down below datetime.timedelta(days=date_window_size)
|
Thanks for this contribution. I added some comments because I was not able to get this working without changes. I ran the code against our test data set (which syncs all tables including the non-async tables) and found no noticeable performance improvement. Any suggestions for figuring out what went wrong? |
|
@siddharthsudheer are you interested in following through with this PR? It seems like it has the potential to make a huge difference if you have small order counts per day. |
No description provided.