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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide a method to do a no-op subscribe to a StreamMessage #4145

Closed
jrhee17 opened this issue Mar 16, 2022 · 4 comments 路 Fixed by #4185
Closed

Provide a method to do a no-op subscribe to a StreamMessage #4145

jrhee17 opened this issue Mar 16, 2022 · 4 comments 路 Fixed by #4185
Milestone

Comments

@jrhee17
Copy link
Contributor

jrhee17 commented Mar 16, 2022

Credit to @trustin for suggesting this idea 馃憤

Some users may want to use StreamMessage#peek to perform some options, but not want the entire message to be in memory (as is done by StreamMessage#collect). The functionality may be similar to Flux#subscribe

In order to achieve this, users may need to do something like the following which can be cumbersome
e.g.

streamMessage.peek(i -> {/* do something */}).subscribe(new Subscriber<Integer>() {
    @Override
    public void onSubscribe(Subscription s) {
        s.request(Long.MAX_VALUE);
    }

    @Override
    public void onNext(Integer integer) {
        // don't keep the messages in memory
    }

    @Override
    public void onError(Throwable t) {}

    @Override
    public void onComplete() {}
});

We could add a method to StreamMessage that performs the above in a convenient matter.

StreamMessage.java

default CompletableFuture<Void> drain() {
    ...
}
@jrhee17 jrhee17 added this to the 1.15.0 milestone Mar 16, 2022
@wooseongshin
Copy link
Contributor

Hello @jrhee17 馃槃 I would like to handle this issue. May I try it?

@jrhee17
Copy link
Contributor Author

jrhee17 commented Mar 16, 2022

All yours @wooseongshin ! Thanks 馃憤

@jrhee17 jrhee17 modified the milestones: 1.15.0, 1.16.0 Mar 21, 2022
@trustin
Copy link
Member

trustin commented Mar 28, 2022

How about naming the method as subscribe(), just like Reactor did?

@wooseongshin
Copy link
Contributor

Thank you for the good suggestion @trustin I'll apply it like that!

Work is delayed due to Corona 馃ぇ I will try to solve it as soon as possible !

@ikhoon ikhoon modified the milestones: 1.16.0, 1.17.0 Apr 15, 2022
trustin pushed a commit that referenced this issue Apr 18, 2022
Motivation:

Provide a method to do a no-op subscribe to a `StreamMessage` #4145

Some users may want to use `StreamMessage#peek` to perform some options, but not want the entire message to be in memory (as is done by `StreamMessage#collect`). The functionality may be similar to `Flux#subscribe`

Modifications:

- Add no-op `subscribe` to `StreamMessage`

Result:

- we can now use no-op `subscribe` for draining data from `StreamMessage`.
- Closes #4145 

Co-authored-by: Ikhun Um <ih.pert@gmail.com>
@trustin trustin modified the milestones: 1.17.0, 1.16.0 Apr 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants