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 11 commits into
base: main
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

@@ -6,6 +6,11 @@
using Microsoft.Rest;
using Microsoft.WindowsAzure.Commands.Common;
using System.Net.Http;
using Azure.Core;
using Azure.Identity;
Copy link
Member

Choose a reason for hiding this comment

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

Are all these required?

Copy link
Author

Choose a reason for hiding this comment

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

Azure.Core and Azure.Identity are required because the TokenCredential and DefaultAzureCredential classes belong to these namespaces, respectively. The remaining unused namespaces have been removed from the code.

@@ -56,7 +56,7 @@ public class DataLakeStoreFileSystemClient
static DataLakeStoreFileSystemClient()
{
// Registering the custom target class
Target.Register<AdlsLoggerTarget>("AdlsLogger"); //generic
LogManager.Setup().SetupExtensions(ext => ext.RegisterTarget<AdlsLoggerTarget>("AdlsLogger"));
Copy link
Member

Choose a reason for hiding this comment

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

What was the reason for this change?

Copy link
Author

Choose a reason for hiding this comment

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

Target.Register(string) has been marked as obsolete. LogManager.Setup().SetupExtensions() method is recommended. This change was introduced in NLog version 5.2 which is a dependency for new adls sdk.

@@ -19,6 +19,7 @@
</ItemGroup>

<ItemGroup>
<PackageReference Include="Microsoft.Identity.Client" Version="4.70.0" />
Copy link
Member

Choose a reason for hiding this comment

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

Unintentional? If this is needed via some transitive dependency, can we add it directly on the datalakestore project?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, this was added unintentionally during testing. Removed it.

@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
@wyunchi-ms
Copy link
Contributor

/azp run

Copy link
Contributor

Azure Pipelines successfully started running 3 pipeline(s).

@wyunchi-ms
Copy link
Contributor

/azp run azure-powershell - powershell-core

Copy link
Contributor

Azure Pipelines successfully started running 1 pipeline(s).

@wyunchi-ms
Copy link
Contributor

/azp run azure-powershell - windows-powershell

Copy link
Contributor

Azure Pipelines successfully started running 1 pipeline(s).

@wyunchi-ms
Copy link
Contributor

/azp run

Copy link
Contributor

Azure Pipelines successfully started running 3 pipeline(s).

@wyunchi-ms
Copy link
Contributor

/azp run

Copy link
Contributor

Azure Pipelines successfully started running 3 pipeline(s).

@wyunchi-ms
Copy link
Contributor

/azp run

Copy link
Contributor

Azure Pipelines successfully started running 3 pipeline(s).

@wyunchi-ms
Copy link
Contributor

/azp run

Copy link
Contributor

Azure Pipelines successfully started running 3 pipeline(s).

@wyunchi-ms
Copy link
Contributor

/azp run

Copy link
Contributor

Azure Pipelines successfully started running 3 pipeline(s).

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

6 participants