Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add page_size arg to list_subscriptions() #1096

Merged
merged 8 commits into from
Jan 31, 2025
Merged

Conversation

asonnenschein
Copy link
Contributor

@asonnenschein asonnenschein commented Jan 29, 2025

This MR adds a new page_size arg to the SubscriptionsClient.list_subscriptions() method. Without specifying a page size, list_subscriptions() will fall back on a default page size of 20 subscriptions per page that is defined in the API view handler. For large numbers of subscriptions, this results in long execution times because list_subscriptions() is only getting 20 subscriptions per page, which results in more requests (and latency in transport) than is necessary.

For example, I tested this out on a sample of 645 subscriptions. The average execution time to get all 645 subscriptions using the default page size of 20 (list_subscriptions()) takes ~36 seconds because list_subscriptions() is making ~33 requests to the API:

(planet-client-python-3.12) ➜  Desktop python example.py
Average execution time: 36.07816209201701 seconds
Average execution time: 37.37259578681551 seconds
Average execution time: 36.77448189887218 seconds
Average execution time: 36.13904633792117 seconds
Average execution time: 35.90926657919772 seconds

When I define a page_size of 700 (list_subscriptions(page_size=700)) the average execution time to get all 645 subscriptions takes only ~3 seconds because list_subscriptions() only has to make one request to the API with the larger page size:

(planet-client-python-3.12) ➜  Desktop python example.py                          
Average execution time: 3.206969585036859 seconds
Average execution time: 2.6358285231981426 seconds
Average execution time: 2.8017055389937013 seconds
Average execution time: 2.6492277118377388 seconds
Average execution time: 2.728695012163371 seconds

Here is the contents of example.py that I used to test and record execution times:

from planet import Session, SubscriptionsClient

async def get_subscriptions():
    async with Session() as sess:
        cl = SubscriptionsClient(sess)
        subscriptions = cl.list_subscriptions(page_size=700, limit=5000)
        sub_list = [i async for i in subscriptions]
        return sub_list

async def main():
    subs = await get_subscriptions()
    return subs

def run_main():
    return asyncio.run(main())

if __name__ == '__main__':
    import asyncio
    import timeit
    execution_times = timeit.repeat(run_main, number=10, repeat=5)
    for execution_time in execution_times:
        print(f"Average execution time: {execution_time} seconds")

@asonnenschein asonnenschein changed the title Add page_size kwarg to list_subscriptions() Add page_size Argument To list_subscriptions() Jan 29, 2025
@asonnenschein asonnenschein changed the title Add page_size Argument To list_subscriptions() Add page_size arg to list_subscriptions() Jan 29, 2025
planet/clients/subscriptions.py Outdated Show resolved Hide resolved
planet/clients/subscriptions.py Outdated Show resolved Hide resolved
@HenryW95
Copy link
Contributor

Would it make sense to add page_size to the CLI for parody with the client?

@asonnenschein
Copy link
Contributor Author

Would it make sense to add page_size to the CLI for parody with the client?

@HenryW95 yes, added!

@HenryW95
Copy link
Contributor

Would it make sense to add page_size to the CLI for parody with the client?

@HenryW95 yes, added!

This docs page also lists the CLI params, so it can be added there: https://github.com/planetlabs/planet-client-python/blob/main/docs/cli/cli-subscriptions.md

@asonnenschein
Copy link
Contributor Author

Would it make sense to add page_size to the CLI for parody with the client?

@HenryW95 yes, added!

This docs page also lists the CLI params, so it can be added there: https://github.com/planetlabs/planet-client-python/blob/main/docs/cli/cli-subscriptions.md

@HenryW95 Updated 👍

planet/cli/subscriptions.py Outdated Show resolved Hide resolved
@asonnenschein asonnenschein merged commit 57add4c into main Jan 31, 2025
9 checks passed
@asonnenschein asonnenschein deleted the adrian/page-size branch January 31, 2025 18:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants