Skip to content

Support pagination for enumerating deleted items #26974

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

Open
wants to merge 17 commits into
base: release-2025-06-03
Choose a base branch
from

Conversation

amrita-hegde
Copy link

@amrita-hegde amrita-hegde commented Jan 7, 2025

Description

Cosmos customers currently utilize the Azure PowerShell for restoring items. However, a limitation exists in the tool: it lacks the capability to retrieve the items by enumerating them page-wise. This change provides the capability to pass a continuation token to the Get-AzDataLakeStoreDeletedItem cmdlet so that, users can retrieve the next page using the continuation token. Maximum allowed value for this numResults(pagesize) is 4000. The number of returned entries could be more or less than numResults.

###Changes

Azure PowerShell

To enable subsequent pages to be fetched from PowerShell,

Define the optional parameter Continuation Token in Azure PowerShell for GetAzureRmDataLakeStoreDeletedItem command.

Use the continuation token to fetch subsequent pages when calling the API.

.NET SDK

Azure/azure-data-lake-store-net#69
Change is released in https://www.nuget.org/packages/Microsoft.Azure.DataLake.Store/2.0.2

This overloaded method will be called from Azure PowerShell by consuming the latest SDK version 2.0.2.

Testing

Screenshot 2025-04-21 184949

Response from GetAzureRmDataLakeStoreDeletedItem cmd

image

Test execution

Screenshot 2025-03-13 165029

When expired/incorrect continuation token is passed

Screenshot 2025-03-13 114340

Local build

image

Mandatory Checklist

  • SHOULD update ChangeLog.md file(s) appropriately
    • For SDK-based development mode, update src/{{SERVICE}}/{{SERVICE}}/ChangeLog.md.
      • A snippet outlining the change(s) made in the PR should be written under the ## Upcoming Release header in the past tense.
    • For autorest-based development mode, include the changelog in the PR description.
    • Should not change ChangeLog.md if no new release is required, such as fixing test case only.
  • SHOULD regenerate markdown help files if there is cmdlet API change. Instruction
  • SHOULD have proper test coverage for changes in pull request.
  • SHOULD NOT adjust version of module manually in pull request

Sorry, something went wrong.

Copy link

Thanks for your contribution! The pull request validation has started. Please revisit this comment for updated status.

@amrita-hegde
Copy link
Author

amrita-hegde commented Jan 21, 2025 via email

@amrita-hegde amrita-hegde self-assigned this Jan 27, 2025
@amrita-hegde amrita-hegde marked this pull request as ready for review March 17, 2025 05:56
@notyashhh
Copy link
Member

/azp run

Copy link
Contributor

Azure Pipelines successfully started running 3 pipeline(s).

Copy link

This PR was labeled "needs-revision" because it has unresolved review comments or CI failures.
Please resolve all open review comments and make sure all CI checks are green. Refer to our guide to troubleshoot common CI failures.

@amrita-hegde amrita-hegde marked this pull request as draft April 2, 2025 04:28
@amrita-hegde amrita-hegde marked this pull request as ready for review April 21, 2025 17:33
@dolauli
Copy link
Contributor

dolauli commented Apr 22, 2025

/azp run

Copy link
Contributor

Azure Pipelines successfully started running 3 pipeline(s).

@amrita-hegde
Copy link
Author

/azp run

Copy link
Contributor

Commenter does not have sufficient privileges for PR 26974 in repo Azure/azure-powershell

@vidai-msft
Copy link
Contributor

/azp run

Copy link
Contributor

Azure Pipelines successfully started running 3 pipeline(s).

@dolauli dolauli removed this from the Az 13.5.0 (05/06/2025) milestone May 6, 2025
@msJinLei msJinLei self-assigned this May 20, 2025
@msJinLei
Copy link
Contributor

The current issues of the PR

  • Request to upgrading Azure.Core to 1.45.0. My question is It is required? Can you use the 1.44.1?
  • If so we are going to upgrading Azure.Core version
  • Need to depend on DefaultAzureCredential from Azure.Identity. Please don’t depend on Azure.Identity’s authentication interface directly.
  • Failed test cases (can check it after the problems above are resolved)

@amrita-hegde
Copy link
Author

The current issues of the PR

  • Request to upgrading Azure.Core to 1.45.0. My question is It is required? Can you use the 1.44.1?

We can't use Azure.Core version 1.44.1 because this PR requires the latest .NET SDK (2.0.2), which depends on Azure.Core 1.45.0 or higher. You can see the dependency here: https://www.nuget.org/packages/Microsoft.Azure.DataLake.Store/2.0.2#dependencies-body-tab So, updating the version to 1.45.0 is required.

  • Need to depend on DefaultAzureCredential from Azure.Identity. Please don’t depend on Azure.Identity’s authentication interface directly.
    Could you describe your authentication requirement and we can see how to implement your requirement.

Currently, Azure PowerShell uses .NET SDK version 1.2.4-alpha, which authenticates the ADLS client using the ServiceClientCredentials from Microsoft.Rest library. However, this authentication mechanism is no longer supported in the latest SDK version (2.0.2).
The new SDK requires the ADLS client to be instantiated using a TokenCredential from the Azure.Core library. This includes several authentication types (e.g., DefaultAzureCredential, ClientSecretCredential, etc.) provided by the Azure.Identity library. Among these, DefaultAzureCredential is the recommended option. https://learn.microsoft.com/en-us/dotnet/api/azure.core.tokencredential?view=azure-dotnet

  • Failed test cases (can check it after the problems above are resolved)

@msJinLei
Copy link
Contributor

@amrita-hegde The Azure.Core is upgraded to 1.45.0. Please pull the latest changes of main branch to your developing branch.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@msJinLei
Copy link
Contributor

/azp run

1 similar comment
@vidai-msft
Copy link
Contributor

/azp run

@amrita-hegde
Copy link
Author

Updated the implementation for the token acquisition. Kindly review.
But why can't we make use of Azure.Identity interface directly? Why is the DeafultAzureCredential ussge is not recommended?

@msJinLei
Copy link
Contributor

/azp run

Copy link
Contributor

Azure Pipelines successfully started running 3 pipeline(s).

msJinLei
msJinLei previously approved these changes May 27, 2025
@msJinLei msJinLei enabled auto-merge (squash) May 27, 2025 04:41
@msJinLei msJinLei dismissed their stale review May 27, 2025 05:27

CI doesn't pass

@msJinLei
Copy link
Contributor

msJinLei commented May 27, 2025

@amrita-hegde
Please refer to the logs https://dev.azure.com/azclitools/public/_build/results?buildId=247530&view=logs&j=b17395f6-68a3-5682-0476-d3f6f1043109&t=f098cad5-2fd2-534b-df25-06e0723952ab and fix the failed test cases

 Failed!  - Failed:     7, Passed:    12, Skipped:     0, Total:    19, Duration: 7 m 20 s - Microsoft.Azure.PowerShell.Cmdlets.DataLakeStore.Test.dll (net8.0)

@msJinLei msJinLei changed the base branch from main to release-2025-06-03 May 27, 2025 05:31
Copy link

This PR was labeled "needs-revision" because it has unresolved review comments or CI failures.
Please resolve all open review comments and make sure all CI checks are green. Refer to our guide to troubleshoot common CI failures.

auto-merge was automatically disabled May 28, 2025 05:20

Head branch was pushed to by a user without write access

@vidai-msft
Copy link
Contributor

/azp run

Copy link
Contributor

Azure Pipelines successfully started running 3 pipeline(s).

return new object();
}
)
Account = AzureRmProfileProvider.Instance.Profile.DefaultContext.Account,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have 2 modes: recording and playback. In playback mode, you should use MockCredential. Specifically in you case, you should set AdlsClientFactory.IsTest = true. In pipeline, the test cases are all run in playback mode, which no access to EntraID service.

@isra-fel isra-fel force-pushed the release-2025-06-03 branch from 85ea2c5 to ba9dc3f Compare May 29, 2025 03:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants