Skip to content
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

Add support for abort signal for AwaitableSender.send() #48

Closed
amarzavery opened this issue Jun 28, 2019 · 7 comments
Closed

Add support for abort signal for AwaitableSender.send() #48

amarzavery opened this issue Jun 28, 2019 · 7 comments

Comments

@amarzavery
Copy link
Collaborator

  • Add support for abort signal for AwaitableSender.send().
  • Should this also be enabled for creating/closing a connection, sender, receiver, session?
  • Should we be using @azure/abort-controller package over here? If yes, then that would mean taking a dependency on something in @azure scope for a generic project like this. It may be ok in the end. However, we should see if there are other alternatives before doing that.
@amarzavery
Copy link
Collaborator Author

cc @ramya-rao-a

@ramya-rao-a
Copy link
Collaborator

@bterlson any thoughts on alternatives to @azure/abort-controller as mentioned above? I believe all we need is the AbortSignalLike interface

@ramya-rao-a
Copy link
Collaborator

cc @ShivangiReja

@bterlson
Copy link

bterlson commented Jul 3, 2019

I don't think a dependency on @azure/abort-controller is appropriate here. As you just need the AbortSignalLike interface, I'd say just roll with that. It only needs to be as complete as what you use internally I think (e.g. if you only use onabort, something like { onabort(cb: () => void): void } might be sufficient even...

@amarzavery
Copy link
Collaborator Author

That is not true, If you take a look at the usage of abort controller in the EventHubs Sender over here. You will see that we are checking for the aborted property. Which means we will have to pretty much re-implement what we have in @azure/abort-controller internally in rhea-promise or create a separate package.

@bterlson - How about moving abort-controller to https://github.com/ts-common and publish it under the @ts-common scope? That is pretty generic. It is open source and any one can use it.

@bterlson
Copy link

bterlson commented Jul 5, 2019

In the example you linked, all you need is the AbortSignalLike type I think (that's what its using anyway)? I'm not sure what would require you to have the whole implementation, can you explain more?

@ramya-rao-a
Copy link
Collaborator

AbortSignal support is now available in all the async operations in the latest release of 1.1.0!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants