[Storage] Added HNS Soft Delete #19753
Conversation
seanmcc-msft
requested review from
KrzysztofCwalina,
amishra-dev,
amnguye,
kasobol-msft and
tg-msft
as code owners
ghost
added
the
| ``` yaml | ||
| input-file: | ||
| - https://raw.githubusercontent.com/Azure/azure-rest-api-specs/bc0a3368a4e8ff55ed69dc498e69437ec92cf0b1/specification/storage/data-plane/Microsoft.StorageDataLake/stable/2020-02-10/DataLakeStorage.json | ||
| - C:\Users\seanmcc\git\azure-rest-api-specs\specification\storage\data-plane\Microsoft.StorageDataLake\stable\2020-06-12\DataLakeStorage.json |
There was a problem hiding this comment.
TODO change this one Azure/azure-rest-api-specs#11741 has merged
|
/azp run net - storage - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| List<ListBlobsIncludeItem> include = new List<ListBlobsIncludeItem> | ||
| { | ||
| ListBlobsIncludeItem.Deleted | ||
| }; |
There was a problem hiding this comment.
this doesn't seem to be passed anywhere.
| /// A <see cref="RequestFailedException"/> will be thrown if | ||
| /// a failure occurs. | ||
| /// </remarks> | ||
| public virtual Response<DataLakePathClient> RestorePath( |
There was a problem hiding this comment.
should this be UndeletePath (to be consistent with UndeleteFileSystem)?
|
/azp run net - storage - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| response = await BlobFileSystemRestClient.ListBlobHierarchySegmentAsync( | ||
| delimiter: null, | ||
| prefix: path, | ||
| marker: continuation, | ||
| maxResults: maxResults, | ||
| include: null, | ||
| timeout: null, | ||
| cancellationToken: cancellationToken) | ||
| .ConfigureAwait(false); |
There was a problem hiding this comment.
I find it odd that we're not passing showOnly anywhere here but I can see showOnly being correctly sent in the recording. How does it work?
Could you add test where we have 1-2 not-deleted files and 1-2 deleted files and make sure normal listing and list deleted return right entries?
There was a problem hiding this comment.
showOnly is hardcoded in swagger.
This API is only used to list deleted paths, normal listing is not an option. Normal listing is accomplished with DataLakeFileSystemClient.GetPaths().
I can expand the unit test for this API.
There was a problem hiding this comment.
can showOnly be unhardcoded ? or generated api name changed? this will be confusing to anybody revisiting this code in the future.
There was a problem hiding this comment.
We will be throwing this all away in less than 6 months when we migrate Data Lake to the Blob endpoint. When I add this functionality to the Blob swagger, it will not be hardcoded.
There was a problem hiding this comment.
Please add a comment in the code explaining the situation then.
| /// <summary> | ||
| /// Gets a prefix, relative to the delimiter used to get the paths. | ||
| /// </summary> | ||
| public string Prefix { get; internal set; } |
There was a problem hiding this comment.
@kasobol-msft, since we are not exposing the delimiter parameter, I don't think it makes sense for us to include Prefix and IsPrefix on PathHierarchyDeletedItem, since they will always be null and false. It might make sense to remove this class all together, and have PathDeletedSegment have an IEnumerable<PathDeletedItem> instead.
|
/azp run net - storage - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run net - storage - tests |
|
/azp run net - storage - ci |
|
Azure Pipelines successfully started running 1 pipeline(s). |
1 similar comment
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run net - storage - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
3 participants
Continued from #17054