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

Reactive Postgres Client quickstart #130

Merged
merged 4 commits into from
May 15, 2019

Conversation

tsegismont
Copy link
Contributor

A sample RESTful application using the Reactive Pg Client to manage fruit entities.

Copy link
Member

@FroMage FroMage left a comment

Choose a reason for hiding this comment

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

We already have a demo for the reactive-pg-client (using rxjava not axle) at https://github.com/jbossas/protean-rest-http-crud-demo/tree/master/quarkus-vertx/src/main/java/io/quarkus

I think we should change our demo to make it use axle, but I think it's cleaner if we keep the DB operations inside the Fruit class like I did. WDYT?

Copy link
Member

@cescoffier cescoffier left a comment

Choose a reason for hiding this comment

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

I'm a bit puzzled by the different experience it's providing in comparison to Panache. I believe some integration work is required.

@tsegismont
Copy link
Contributor Author

We already have a demo for the reactive-pg-client (using rxjava not axle) at https://github.com/jbossas/protean-rest-http-crud-demo/tree/master/quarkus-vertx/src/main/java/io/quarkus

GH says not found when I click on this link.

I think we should change our demo to make it use axle, but I think it's cleaner if we keep the DB operations inside the Fruit class like I did. WDYT?

I don't think about the Fruit class as a domain object (it's more a transfer object as a matter of fact). But I have no preference. If you want to keep the style consistent it's fine with me.

@FroMage
Copy link
Member

FroMage commented Apr 3, 2019

I've asked for permission to be granted to you to look at it. Then we can debate which one looks better. My guess is that we may end up with a mix of the two.

@tsegismont
Copy link
Contributor Author

I'm a bit puzzled by the different experience it's providing in comparison to Panache. I believe some integration work is required.

It's not perfect. As you said, an extension for the Reactive Pg Client would be an improvement. However the configuration bean uses Quarkus tech (Arc, Microprofile) so it's not terrible either.

@tsegismont
Copy link
Contributor Author

I've asked for permission to be granted to you to look at it. Then we can debate which one looks better. My guess is that we may end up with a mix of the two.

I had a look and it's indeed the same app with 2 differences:

  • in this PR, the Axle shim is used, the demo uses RxJava
  • in this PR, the data access code is not internal to the entity.

As explained in the "Using Vert.x" guide, the Axle shim works nicely with the rest of Quarkus and MP API. So I believe it is better to use it in the guide.

As for data access code in the entity, if it's a blocker I'll move do the changes here, but I believe this style would better a fit a Reactive Panache guide.

@FroMage
Copy link
Member

FroMage commented Apr 3, 2019

We should totally use Axle and I should update the http-rest-client demo to that too, agreed.

As for the SQL code location, I really think it should go in the entity to keep the model and controller logic separated.

@FroMage
Copy link
Member

FroMage commented May 9, 2019

LGTM, just two minor questions.

@tsegismont
Copy link
Contributor Author

@FroMage I've updated the PR.

@FroMage
Copy link
Member

FroMage commented May 13, 2019

OK thanks. @cescoffier do you still have comments blocking this merge or shall we merge it?

Copy link
Member

@cescoffier cescoffier left a comment

Choose a reason for hiding this comment

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

I just made a very very small comment. The rest looks good.

@cescoffier cescoffier merged commit 1429a8e into quarkusio:development May 15, 2019
@cescoffier cescoffier deleted the reactive-pg-client branch May 15, 2019 18:46
@cescoffier
Copy link
Member

and it's merged!

@tsegismont
Copy link
Contributor Author

tsegismont commented May 15, 2019 via email

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

Successfully merging this pull request may close these issues.

3 participants