Skip to content

[Storage] forceCloseAllHandles on FileClient and DirectoryClient#5620

Merged
ramya-rao-a merged 4 commits into
Azure:masterfrom
HarshaNalluru:ForceCloseAllHandles
Oct 18, 2019
Merged

[Storage] forceCloseAllHandles on FileClient and DirectoryClient#5620
ramya-rao-a merged 4 commits into
Azure:masterfrom
HarshaNalluru:ForceCloseAllHandles

Conversation

@HarshaNalluru
Copy link
Copy Markdown
Contributor

@HarshaNalluru HarshaNalluru commented Oct 17, 2019

This is almost same as @JoshLove-msft's equivalent PR Azure/azure-sdk-for-net#8134 in .NET

@HarshaNalluru HarshaNalluru requested a review from xirzec October 17, 2019 05:34
@HarshaNalluru HarshaNalluru added Client This issue points to a problem in the data-plane of the library. Storage Storage Service (Queues, Blobs, Files) labels Oct 17, 2019
* @returns {Promise<number>}
* @memberof FileClient
*/
public async forceCloseAllHandles(options: FileForceCloseHandlesOptions = {}): Promise<number> {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why not make it pageble like other segmement APIs?

An internal loop may cause "unresponsive" for large number of handles.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In general we don't add paged async iterators for operations that do actions but not retrieving data. /cc @bterlson

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This was discussed and in addition to @jeremymeng's point, it was decided that, given that this is a super niche use case, doing a simpler thing is acceptable.

* @memberof DirectoryClient
*/
public async forceCloseAllHandles(
options: DirectoryForceCloseHandlesSegmentOptions = {}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We also want a boolean recursive parameter. Is it supported by DirectoryForceCloseHandlesSegmentOptions or do we have to navigate the directory structure by ourselves?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

recursive parameter is present in DirectoryForceCloseHandlesSegmentOptions.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good.

@ramya-rao-a
Copy link
Copy Markdown
Contributor

Fixes #5544

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Client This issue points to a problem in the data-plane of the library. Storage Storage Service (Queues, Blobs, Files)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants