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 reactive mongodb sample #9761

Closed
wants to merge 6 commits into from
Closed

Add reactive mongodb sample #9761

wants to merge 6 commits into from

Conversation

rajadilipkolli
Copy link
Contributor

adds spring-boot-data-mongdb-responsive sample using new spring data mongodb reactive starter

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 14, 2017
@snicoll snicoll changed the title spring-boot-data-mongdb-responsive sample Add reactive mongodb sample Jul 14, 2017
changed to flux to showcase reactive nature
@mp911de
Copy link
Member

mp911de commented Jul 17, 2017

I'm not sure about the example.

The code shows reactive API usage within a synchronous piece of code (CommandLineRunner) and heavily makes use of .block() methods. I think an example might make more sense in a different context, such as WebFlux or inserting data asynchronously (Flux.interval(…)).

@rajadilipkolli
Copy link
Contributor Author

@mp911de agreed that block shouldn't be used. what is the reactive equivalent of CommandLineRunner ?

I have used this in my WebFlux project and copy pasted from there,hence it makes sense there but not here.

@philwebb philwebb added the status: waiting-for-feedback We need additional information before we can continue label Jul 17, 2017
@rajadilipkolli
Copy link
Contributor Author

@mp911de Can you please review my latest changes and let me know if I need to improve on anything in this PR

@snicoll snicoll added the for: team-attention An issue we'd like other members of the team to review label Jul 26, 2017
@snicoll
Copy link
Member

snicoll commented Jul 26, 2017

I am not sure we should add a reactive/non reactive variant of each spring data sample, especially considering how the setup is similar. It feels to me that a Spring Data sample is a better place for it and perhaps such sample already exists.

Let's see what the rest of the team thinks.

@mp911de
Copy link
Member

mp911de commented Jul 26, 2017

We have a few samples in our Spring Data Examples repository. We aren't entirely happy with our current samples because the majority of environments (CommandLineRunner, JUnit tests) follows an imperative programming model which is a limiting factor. An end-to-end reactive flow isn't possible without the appropriate runtime environment thus we are forced to use blocking calls or test facilities (such as StepVerifier).

A reactive sample would make sense in the context of WebFlux, but then it's no longer a pure MongoDB example.

@philwebb philwebb added the status: on-hold We can't start working on this issue yet label Jul 26, 2017
@bclozel
Copy link
Member

bclozel commented Aug 30, 2017

I agree with @snicoll, there's no point in adding all the possible variants out there, especially when the setup is so similar.

Now about the CommandLineRunner, you definitely have a point.
I've seen a lot of code samples out there just calling Flux.subscribe(Consumer) — this won't guarantee that the processing of that Flux is done before the application is shut down/the webapp is started.

We could create something like:

@FunctionalInterface
public interface ReactiveCommandLineRunner {
  Mono<Void> run(String... args);
}

At processing time, we would need to concat all of them (to guarantee a processing order) and block, like:

firstMono.concatWith(secondMono).then().block();

What do you think?

@snicoll
Copy link
Member

snicoll commented Aug 30, 2017

@bclozel thanks for the reminder. Can we move the CommandLineRunner conversation to a dedicated issue? In the meantime, we're going to close this one as the Spring Data samples are already covering this.

@snicoll snicoll closed this Aug 30, 2017
@snicoll snicoll added status: declined A suggestion or change that we don't feel we should currently apply and removed for: team-attention An issue we'd like other members of the team to review status: on-hold We can't start working on this issue yet status: waiting-for-feedback We need additional information before we can continue status: waiting-for-triage An issue we've not yet triaged labels Aug 30, 2017
@rajadilipkolli rajadilipkolli deleted the patch-2 branch August 30, 2017 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants