Implement cursor pagination

[emdemir]

fixes #337

This PR implements cursor pagination which was introduced with the 2019-07 version of the Shopify API by implementing a new subclass of pyactiveresource.Collection which is able to handle the cursor pagination headers sent with the response. The returned object from ShopifyResource.find() has next and previous methods and by default caches pages. This PR also introduces a PaginatedIterator class which is used to iterate over a large number of pages in a memory-efficient manner as it only keeps the current page in memory.

Usage examples:

# iterate over all products
import shopify
products = shopify.Product.find()
for item in products:
  # this will fetch next pages and iterate over them as necessary,
  # thereby iterating over every product
  do_something_with_product(item)

# get first 2 pages of products with custom arguments
page_one = shopify.Product.find(limit=100)
page_two = page_one.next(limit=25)

# iterate with PageIterator
for page in shopify.PageIterator(shopify.Product.find()):
  for item in page:
    do_something_with_product(item)

This PR depends on Shopify/pyactiveresource#29 so I have currently set it as a draft PR. Once that is merged I will un-draft this one.

[h55nick]

OMG savior! Just ran into a situation where I was thinking this would be super helpful.

Looks like there is an import error causing some havoc on the test suite.

🎉 #opensource

I'll be patiently awaiting this merge.

[emdemir]

@h55nick The error is due to my changes to pyactiveresource (Shopify/pyactiveresource#29) not having been merged yet. If you download and vendor my branches like this;

• add https://github.com/emdemir/shopify_python_api and https://github.com/emdemir/pyactiveresource as submodules to your repo
• checkout the branches I'm doing a PR from
• add them to your PYTHONPATH at the start of your script:

sys.path.insert(1, VENDOR_PATH + "/shopify_python_api")
# (same for activeresource)

Now you should be able to use my code.

[tanema]

I will get someone to update the release of pyactiveresource so that you can update this PR so that the tests pass.

[garethson]

Looking great, thanks so much!

Does this support the case where we have params in the URL, not just querystring? For example: collection_listings/<collection id>/product_ids.json

This was caught and fixed in our Ruby gem for this feature here, and it relies on simply passing the next/prev links into the from param, which it looks like pyresource does have, which is good.

It does mean that'd you'd probably (?) have to drop the ability to pass in variables to the extra_params in your next/previous methods here. IMO that's fine, as I don't actually see that many valid use cases for wanting to find next/previous pages with new param. You also could then drop the params parsing as well.

[GhostApps]

Any update on this? I'm getting emails from Shopify saying that I'm making calls to an out-of-date API

[philpax]

We've successfully used this PR in our own code, and are looking forward to seeing it merged into upstream.

One thing that would be nice to see is having PaginatedIterator implicitly handle non-paginated collections, so that code that iterates over pages works regardless; I've had to use this workaround, which is a bit ugly:

# shopify.PaginatedIterator only works if the collection in question is paginated.
# If there is only one page, the collection will not be paginated.
# To work around this, we only use the PaginatedIterator if the collection is actually
# a PaginatedCollection.
iterator = PaginatedIterator(collection) if isinstance(collection, PaginatedCollection) else (collection,)

[GhostApps]

Any update on this? I'm getting emails from Shopify saying that I'm making calls to an out-of-date API

Bumping this

[stlk]

Hi, I'm sorry but I believe this should get higher priority.

[tylerball]

@tanema @emdemir pyactiveresource 2.2.0 has been released.

[UnicycleJonathan]

Bumping this. Shopify folks, are you kidding me? How can expect Python developers to do a mandatory migration to cursor-based pagination when your own SDK doesn't support it? Please get this done ASAP.

Shopify has a long history of doing developer-unfriendly things like this and it's been really frustrating.

[philpax]

Ditto on the bump - we've been using the fork for several months now and would prefer to be using mainline.

[sprataa]

just joining the bump party here

[tanema]

@UnicycleJonathan I understand that you are frustrated however you need to understand we are not doing anything maliciously against you. For reasons that are out of most of our hands we have not been able to get to this yet. Please watch how you speak to our developers or your repo access will be revoked

[travis-st]

I second what @UnicycleJonathan says. There's no reason @tanema should threaten revoking access for making a valid point.

We just received another complaint email from Shopify:

One or more of your apps have made deprecated API calls in the last 30 days. Support for these versions will be removed on April 1, 2020. Please update the apps listed below to API version 2019-07 or later to ensure they continue to function correctly:

[tanema]

I ask you to consider if you ran a department store and all of your employees were doing their best, to offer their best. You then had a customer, unsatisfied come in and talk to you and your employees as @UnicycleJonathan has talked to me and my coworkers. Would you continue to welcome that person into your workplace?

I understand that you are all frustrated, and I am as well as I would really like to get this finished and not have to talk about it anymore however I do not have the resources to do so currently.

I have no qualms about revoking access to developers if it means protecting the mental health of my coworkers. Everyone is doing their best and no one deserves to be talked to in such a manner.

[travis-st]

I think the analogy would be more accurate if instead of customers, we considered a vendor that sells merchandise to your department store. You see, the vendor runs a small business, maybe 5 employees, and greatly benefits from the merchandise it sells at your department store. Meanwhile, the department store, at last count, had over 4,000 employees.

Then one day, the department store determines that the only way to deliver the merchandise is to deliver it through the door on the northwest corner of the building. But that door doesn't work, so he has to go through the southwest door. But in doing so, the vendor receives repeated warnings and reprimands from the department store about using said door. The department store says that it's too busy to fix the northwest door though, and that the vendor will just have to wait, and endure the reprimands and warnings for using the southwest door.

[UnicycleJonathan]

Hi @tanema, I apologize if my message caused anxiety or offense for your and your team. I certainly didn't intend to, and it definitely wasn't personal, I don't know any individuals on this team and didn't direct anything towards any individual, my frustration is simply with the company. I understand what it feels like to be overcommitted and under-resourced and how upsetting it can be when facing criticism when you're doing your best with what you have.

Regardless of how it came to be, the situation for developers with regards to this migration is unacceptable and distressing. Shopify has demanded that developers make a change but then withheld the tools required to make that change. And all the while a deadline approaches, a ticking time bomb after which our applications will break and our customers (which are also your customers) will be massively impacted.

This is clearly not deliberate or malicious and I would never suggest that it was. We can only speculate from the outside, but it's probably the result of challenges in communication, staffing, and prioritization across disparate arms of a large organization. But it doesn't matter, it's not an acceptable situation to put developers into. Shopify simply needs to either extend deadlines under these circumstances, or to have postponed these breaking changes to a future API version when all the SDKs would be ready.

The final remark in my previous message was meant to highlight that this isn't the first time we as developers have been put into these kinds of fraught situations by Shopify. When this kind of stressful situation happens repeatedly it can feel like Shopify doesn't quite understand what it feels like to be a developer on the platform or how to provide the tools and support to developers for them to succeed.

When we express this deep and genuine frustration and then are threatened with a ban in response, it would seem to prove that point.

[tanema]

Well This discussion is no longer on topic and is no longer constructive. Instead of banning I am locking this conversation. I will open another PR to do this work when I have time.

[tanema]

Completed here #352