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

Unable to use $top with list users #13625

Closed
Stono opened this issue Feb 5, 2021 · 8 comments
Closed

Unable to use $top with list users #13625

Stono opened this issue Feb 5, 2021 · 8 comments
Assignees
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. Graph needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team question The issue doesn't require a change to the product in order to be resolved. Most issues start as that

Comments

@Stono
Copy link

Stono commented Feb 5, 2021

Describe the bug
Hi,
I can't for the life of me get $top working when listing users.

  ): Promise<Array<User>> {
    const client = await this.getClient()
    const options = { $top: '10' }
    const users = odatanextLink
      ? await client.users.listNext(odatanextLink, options)
      : await client.users.list(options)

    if (users.odatanextLink) {
      const aggregatedUsers = runningUserList.concat(users)
      console.log(users.odatanextLink, aggregatedUsers.length)
      return this.users(aggregatedUsers, users.odatanextLink)
    }
    return runningUserList

When i run this code, i always get a page size of 100 regardless what value i set '$top' to. Can you see what i'm doing wrong?

@ghost ghost added needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. customer-reported Issues that are reported by GitHub users external to the Azure organization. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Feb 5, 2021
@ramya-rao-a
Copy link
Contributor

Hey @Stono

Can you share which package are you using here? This repo houses multiple packages, and it would help to know which one are you using.

@Stono
Copy link
Author

Stono commented Feb 5, 2021

Sorry, @azure/graph specifically GraphRbacManagementClient

@ramya-rao-a
Copy link
Contributor

Thanks @Stono

I don't see $top being a valid option to set in either the list or listNext methods.
This would mean that you can only ever get whatever the service returns being in the next page and not really control the page size.

I may be wrong here...

@joheredi, This package is auto generated using the v4 code gen. Can you share if there is any way to control the page size?

@ramya-rao-a ramya-rao-a added Client This issue points to a problem in the data-plane of the library. Graph labels Feb 5, 2021
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Feb 5, 2021
@ghost ghost added the needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team label Feb 5, 2021
@deyaaeldeen
Copy link
Member

@ramya-rao-a Generally, in our core-paging package, we have a byPage method

byPage: (settings?: PageSettingsT) => AsyncIterableIterator<PageT>;
which should be part of every paged iterator in our SDK. This method takes settings as input which by default is an object with maxPageSize field that controls the size of the page.

Correct me if I am wrong @joheredi but I think this paging support was recently implemented in the code generator v6 and this package was not generated in a long time.

@ramya-rao-a
Copy link
Contributor

@deyaaeldeen This issue is for the @azure/graph package which uses @azure/ms-rest-js and @azure/ms-rest-azure-js, not any of the newer core packages. So, the above note on core-paging does not apply

@deyaaeldeen
Copy link
Member

@Stono I created a fix but I would like to verify that it works for you. Could you please run this to create the patched package locally?
git clone --no-checkout https://github.com/azure/azure-sdk-for-js graphrbac_patched && cd graphrbac_patched && git fetch origin pull/13858/head:pr/13858 && git checkout pr/13858 && git sparse-checkout init --cone && git sparse-checkout set sdk/graphrbac/graph && cd sdk/graphrbac/graph && npm pack

The package will be created in this path: <the path to your current directory before running the command>/graphrbac_patched/sdk/graphrbac/graphazure-graph-5.1.0.tgz. You can then install it using npm install <path to the tgz file> inside your local project.

@Stono
Copy link
Author

Stono commented Feb 19, 2021

Hello
Thanks for doing this, but we've moved to the @microsoft/microsoft-graph-client client now (interestingly top doesn't seem to work there either - haha).

@ramya-rao-a
Copy link
Contributor

Thanks for getting back to us on this @Stono

Closing this issue as per #13858 (comment)

openapi-sdkautomation bot pushed a commit to AzureSDKAutomation/azure-sdk-for-js that referenced this issue Mar 25, 2021
add packageversion (Azure#13625)

Co-authored-by: Yan Zhang (WICRESOFT NORTH AMERICA LTD) <v-yanzhang@microsoft.com>
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. Graph needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team question The issue doesn't require a change to the product in order to be resolved. Most issues start as that
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants