Skip to content

Options to cancel a Task. #128

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

Closed
fabianfett opened this issue Nov 12, 2019 · 8 comments
Closed

Options to cancel a Task. #128

fabianfett opened this issue Nov 12, 2019 · 8 comments
Labels
kind/enhancement Improvements to existing feature.

Comments

@fabianfett
Copy link
Member

fabianfett commented Nov 12, 2019

Hi, the AsyncHTTPClient currently has an interface, that makes the consumer choose between

  1. shorthands to get/post/execute, where the consumer doesn’t have to implement the delegate but has no chance to cancel a task
  2. full access to the task but one has to implement the delegate.

I would like to be able to cancel my request but I don’t want to implement the delegate protocol myself. Currently the ResponseAccumulator is an internal class and therefore can’t be used by a consumer.

Would it be possible to make the ResponseAccumulator public? Or can we build an execute method that doesn't need a delegate but returns a Task instead of a Response?

Whatever the preferred way is, if there is a decision on how to move forward, I'd be interested in creating a potential PR.

@weissi weissi added the kind/enhancement Improvements to existing feature. label Nov 14, 2019
@Yasumoto
Copy link
Contributor

Returning a Task seems like a better interaction than having to plug in your own ResponseAccumulator, but I'd be interested to hear if you've had the chance to try that out!

@artemredkin
Copy link
Collaborator

There is a small problem with returning a task - it will change public API :(, though I do agree that its a better solution...

@fabianfett
Copy link
Member Author

@artemredkin So what's your preferred way forward? Are you open for a breaking change when switching to 2.0? Though I'm aware that we just recently got to 1.0. What do we want to do in the meantime? Exposing a second endpoint?

@vkill
Copy link
Contributor

vkill commented Jan 17, 2020

ResponseAccumulator public and final +1

@krzyzanowskim
Copy link
Contributor

ResponseAccumulator public and not final +1

@Lukasa
Copy link
Collaborator

Lukasa commented Jan 20, 2020

I think making ResponseAccumulator public is a good idea. I'm inclined to begin making it public and not open and to give it no public API surface area.

I believe @krzyzanowskim wants ResponseAccumulator to be non-final to adapt it to a streaming model. I don't think that's a particularly good idea: the streaming use-case should be served by HTTPClientCopyingDelegate once #154 ships.

artemredkin pushed a commit that referenced this issue Jan 20, 2020
As discussed in #128. We make the ResponseAccumulator public to give developers an easy time to create a Task. With the ResponseAccumulator the developer using this does not have to worry, about implementing the HTTPClientResponseDelegate all by himself.
@weissi
Copy link
Contributor

weissi commented Feb 26, 2020

@fabianfett can we close this issue?

@fabianfett
Copy link
Member Author

@weissi Yes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Improvements to existing feature.
Projects
None yet
Development

No branches or pull requests

7 participants