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

Missing Pagination for List of Directs #612

Open
temporaer opened this issue Oct 28, 2024 · 9 comments
Open

Missing Pagination for List of Directs #612

temporaer opened this issue Oct 28, 2024 · 9 comments
Labels
area:documentation Focused on documentation of the product documentation Improvements or additions to documentation type:documentation General documentation request or project documentation update

Comments

@temporaer
Copy link

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 temporaer added status:waiting-for-triage An issue that is yet to be reviewed or assigned type:bug A broken experience labels Oct 28, 2024
@temporaer
Copy link
Author

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

@baywet
Copy link
Member

baywet commented Nov 19, 2024

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 baywet added documentation Improvements or additions to documentation type:documentation General documentation request or project documentation update area:documentation Focused on documentation of the product and removed status:waiting-for-triage An issue that is yet to be reviewed or assigned type:bug A broken experience labels Nov 19, 2024
@temporaer
Copy link
Author

temporaer commented Dec 25, 2024

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

@baywet
Copy link
Member

baywet commented Dec 27, 2024

@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.

@temporaer
Copy link
Author

@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.

@baywet
Copy link
Member

baywet commented Dec 27, 2024

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.
microsoft/kiota#1569

@temporaer
Copy link
Author

@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
Copy link
Author

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?

@baywet
Copy link
Member

baywet commented Dec 27, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:documentation Focused on documentation of the product documentation Improvements or additions to documentation type:documentation General documentation request or project documentation update
Projects
None yet
Development

No branches or pull requests

2 participants