msgraph-beta-sdk-python icon indicating copy to clipboard operation
msgraph-beta-sdk-python copied to clipboard

Missing Pagination for List of Directs

Open temporaer opened this issue 1 year ago • 9 comments

Describe the bug

I am fetching directs via client.users.by_user_id(user_id).direct_reports.get(). The list is incomplete for users with many directs. Browsing through the code a bit, it seems that this API ignores the nextLink property and only returns the first page.

Expected behavior

The list of returned directs should be complete.

How to reproduce

  • create user user_id with many directs
  • fetch direct reports of this user with client.users.by_user_id(user_id).direct_reports.get()
  • confirm not all results are returned.

SDK Version

1.11.0

Latest version known to work for scenario above?

none.

Known Workarounds

No response

Debug output

Click to expand log ```
</details>


### Configuration

- OS: Ubuntu 22.04
- arch: x86


### Other information

_No response_

temporaer avatar Oct 28 '24 10:10 temporaer

@samwelkanda @baywet -- tagging some regulars here, are you guys monitoring the issues list? Thanks!

temporaer avatar Nov 19 '24 08:11 temporaer

Hi @temporaer Thank you for using the SDK and for reaching out.

You can use the page iterator to page through the different pages automatically.

Unfortunately, there's no documentation sample for Python yet, but using the other languages and reading the source you should be able to extrapolate. CC @jasonjoh

Let us know if you have any additional comments or questions.

baywet avatar Nov 19 '24 12:11 baywet

@baywet , @jasonjoh, why is this not a bug? Why isn't pagination implemented without user intervention like for other list operations?

temporaer avatar Dec 25 '24 20:12 temporaer

@temporaer depending on the application and requirements, some scenarios might require a full enumeration of all the elements (e.g. auditing data sets), some might need to stop mid-way through (e.g. user clicking a UI and paging through), and others might only need the first page (e.g. recap of the last few elements).

We cannot assume that every application out there is going to need all items of the collection, that'd put unnecessary burden on the service and the client applications for scenarios where it's not needed.

Let us know if you have any additional comments or questions.

baywet avatar Dec 27 '24 13:12 baywet

@baywet , you could return an iterator instead of a list, as is common practice. Then, each application can decide whether to fetch the next page, without adding unnecessary burden on the service.

temporaer avatar Dec 27 '24 13:12 temporaer

We did discuss that in the early design of this new generation of SDKs and decided to keep things simple for now. Here is more context in case you're interested. https://github.com/microsoft/kiota/issues/1569

baywet avatar Dec 27 '24 14:12 baywet

@baywet , I see. That's a very different argument though. In this case, please make sure this limitation is clearly documented, and a (potentially more involved) way to list all directs is provided. Thank you!

temporaer avatar Dec 27 '24 14:12 temporaer

I'm not super sure about the linked context -- it fseems that there, the work was completed, but it doesn't seem to explain why it wasn't picked up in the newer version of the SDK. Is there generally no pagination support, or are there certain list operations that you left out intentionally?

temporaer avatar Dec 27 '24 14:12 temporaer

Ah sorry, I assumed the context had been shared from internal discussions. The gist being: we needed a way to generate paging for the CLI, as layering CLI calls is more complex than in any other language. We were faced with a choice between building that in a bespoke way for the CLI only, or making it generic for any language that kiota supports. In the end, to prioritize the release of the CLI, it was decided to take the shortest path, and NOT make things generic for other languages to include an additional "getAll" method (in addition to get) which would include paging.

Let us know if you have any additional comments or questions.

baywet avatar Dec 27 '24 14:12 baywet