okta / okta-sdk-python

Apache License 2.0
229 stars 143 forks source link

OKTA-672843 include response for next() during paging operation #379

Closed bryanapellanes-okta closed 6 months ago

bryanapellanes-okta commented 6 months ago

This change adds a parameter to api_response.next() which causes the response to be returned as the third tuple value during paging operations; this provides access to rate limit headers.

gabrielsroka commented 6 months ago

so, when u first call list_users, it's users, resp, err and the 2nd time it's users, err, resp ???

also, in the example below (from the readme), should it be resp, not response ?

from okta.client import Client as OktaClient
import asyncio

async def main():
    client = OktaClient()
    users, resp, err = await client.list_users()
    while True:
        for user in users:
            print(user.profile.login) 
        if resp.has_next():
            users, err, response = await resp.next(True) # Specify True to receive the response object as the third part of the tuple for further analysis
        else:
            break
justinabrokwah-okta commented 6 months ago

@gabrielsroka Yes, at least we don't break existing scripts for users but now we have the functionality for those who requested

gabrielsroka commented 6 months ago

There are no existing scripts that pass in True. Plus you need to change the tuple and also actually use the response headers which means this code is going to get looked at.

Or you could just create a new method.

also, in the example (from the readme), should it be resp or response ?

gabrielsroka commented 6 months ago

also, since you're adding a new feature, maybe u could take another look at the 2-line PR i submitted last year, which @justinabrokwah-okta approved, but then got rolled back

https://github.com/okta/okta-sdk-python/pull/328

gabrielsroka commented 6 months ago

why is it includeResponse and not include_response?

gabrielsroka commented 6 months ago

in the example (from the readme), should it be resp or response ?

to answer my own q, it looks like u need 2 different variables: resp and resp2. so you can't reuse the get_headers(), either.

if i change resp2 to resp, i get a weird error, it looks like dicts aren't being converted to objects -- idk if this is a bug: AttributeError: 'dict' object has no attribute 'profile'

import okta.client
# import okta.api_response
import asyncio

async def main():
    # async with reuses the session, so it's much faster.
    async with okta.client.Client() as client:
        # Paginate users
        # resp: okta.api_response.OktaAPIResponse
        params = {'filter': 'profile.lastName eq "Doe"', 'limit': 1}
        users, resp, err = await client.list_users(params)
        print(resp.get_headers().get('x-rate-limit-remaining'))
        while True:
            for user in users:
                print(user.profile.login)
            if resp.has_next(): 
                users, err, resp2 = await resp.next(True)
                print(resp2.get_headers().get('x-rate-limit-remaining'))
            else: break

asyncio.run(main())