Skip to content

[Storage] Make Aborter parameter optional - storage-file#2908

Merged
jeremymeng merged 6 commits into
Azure:feature/storagefrom
HarshaNalluru:OptionalAborterParam
May 17, 2019
Merged

[Storage] Make Aborter parameter optional - storage-file#2908
jeremymeng merged 6 commits into
Azure:feature/storagefrom
HarshaNalluru:OptionalAborterParam

Conversation

@HarshaNalluru
Copy link
Copy Markdown
Contributor

Use Aborter.none as the default value.

Issue - #2628
Reference - #2296

@HarshaNalluru HarshaNalluru added Client This issue points to a problem in the data-plane of the library. Storage Storage Service (Queues, Blobs, Files) labels May 15, 2019
@HarshaNalluru HarshaNalluru requested a review from jeremymeng May 15, 2019 01:26
@HarshaNalluru HarshaNalluru self-assigned this May 15, 2019
@jeremymeng jeremymeng requested a review from XiaoningLiu May 15, 2019 03:21
@jeremymeng jeremymeng changed the base branch from master to feature/storage May 15, 2019 03:21
Copy link
Copy Markdown
Member

@jeremymeng jeremymeng left a comment

Choose a reason for hiding this comment

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

can you base you change on top of feature\storage? We will merge updates from master in separate PRs.

Copy link
Copy Markdown
Member

@XiaoningLiu XiaoningLiu left a comment

Choose a reason for hiding this comment

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

Noticed new added options with I prefix, while @jeremymeng is working on remove I prefixes.
Generally looks good to me, please make sure all tests passed.

Why this PR includes changes for other SDKs?

@HarshaNalluru
Copy link
Copy Markdown
Contributor Author

Yup. Ran the tests locally, all the tests have passed.

@HarshaNalluru HarshaNalluru force-pushed the OptionalAborterParam branch from 7e21f59 to ff7666b Compare May 15, 2019 05:14
Comment thread sdk/storage/storage-file/samples/typescript/advanced.ts Outdated
Comment thread sdk/storage/storage-file/samples/typescript/advanced.ts
Comment thread sdk/storage/storage-file/samples/typescript/advanced.ts Outdated
import { appendToURLPath } from "./utils/utils.common";

export interface IDirectoryCreateOptions {
abortSignal?: Aborter;
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.

Need to copy and paste some documentation for abortSignal

Comment thread sdk/storage/storage-file/test/node/highlevel.node.spec.ts Outdated
@jeremymeng
Copy link
Copy Markdown
Member

/azp run azure-sdk-for-js - all-tests - storage-file

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@jeremymeng
Copy link
Copy Markdown
Member

jeremymeng commented May 16, 2019

I have some draft doc for abortSignal, any improvement suggestion? @XiaoningLiu

  /**
   * Aborter instance to cancel request. It can be created with Aborter.none
   * or Aborter.timeout(). Go to documents of {@link Aborter} for more examples
   * about request cancellation.
   *
   * @type {Aborter}
   * @memberof IUploadToBlockBlobOptions
   */
  abortSignal?: Aborter;

Edited: add code snippet type

@XiaoningLiu
Copy link
Copy Markdown
Member

Looks good to me

@jeremymeng jeremymeng merged commit a80cb3b into Azure:feature/storage May 17, 2019
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.

3 participants