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 reactive in the server extension #731

Merged
merged 7 commits into from
Jun 13, 2024

Conversation

carlesarnal
Copy link
Contributor

@carlesarnal carlesarnal commented Jun 12, 2024

Many thanks for submitting your Pull Request ❤️!

Please make sure that your PR meets the following requirements:

  • You have read the contributors guide
  • Your code is properly formatted according to our code style
  • Pull Request title contains the target branch if not targeting main: [0.9.x] Subject
  • Pull Request contains link to the issue
  • Pull Request contains link to any dependent or related Pull Request
  • Pull Request contains description of the issue
  • Pull Request does not include fixes for issues other than the main ticket
This PR introduces support for generating reactive code using the server extension

@carlesarnal carlesarnal linked an issue Jun 12, 2024 that may be closed by this pull request
Copy link
Contributor

@hbelmiro hbelmiro left a comment

Choose a reason for hiding this comment

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

Thank you @carlesarnal.

It seems like you're confusing the new dependencies for the classic and reactive implementations.
See https://quarkus.io/guides/rest-migration#dependencies.

RESTEasy Classic is still known as RESTEasy (quarkus-resteasy)
RESTEasy Reactive now is Quarkus REST (quarkus-rest).

I suggested changes to the pom.xml files.


I also suggest using Maven Profiles to avoid test duplication. With that, you can have just one module and set a flag to run them with each profile (classic or reactive).
We do that in the client module.

That's all you need since all the tests are identical for both implementations.
In case we need specific tests for each implementation in the future, we can use the org.junit.jupiter.api.Tag, like we did here.

Our CI already handles both profiles. See, for example, the job for the reactive profile.

server/integration-tests/reactive/pom.xml Outdated Show resolved Hide resolved
server/integration-tests/reactive/pom.xml Outdated Show resolved Hide resolved
server/integration-tests/reactive/pom.xml Outdated Show resolved Hide resolved
server/integration-tests/reactive/pom.xml Outdated Show resolved Hide resolved
server/integration-tests/resteasy/pom.xml Outdated Show resolved Hide resolved
server/integration-tests/resteasy/pom.xml Outdated Show resolved Hide resolved
carlesarnal and others added 6 commits June 13, 2024 11:08
Co-authored-by: Helber Belmiro <helber.belmiro@gmail.com>
Co-authored-by: Helber Belmiro <helber.belmiro@gmail.com>
Co-authored-by: Helber Belmiro <helber.belmiro@gmail.com>
Co-authored-by: Helber Belmiro <helber.belmiro@gmail.com>
Co-authored-by: Helber Belmiro <helber.belmiro@gmail.com>
Co-authored-by: Helber Belmiro <helber.belmiro@gmail.com>
@carlesarnal
Copy link
Contributor Author

Thank you @carlesarnal.

It seems like you're confusing the new dependencies for the classic and reactive implementations. See https://quarkus.io/guides/rest-migration#dependencies.

RESTEasy Classic is still known as RESTEasy (quarkus-resteasy) RESTEasy Reactive now is Quarkus REST (quarkus-rest).

I suggested changes to the pom.xml files.

I also suggest using Maven Profiles to avoid test duplication. With that, you can have just one module and set a flag to run them with each profile (classic or reactive). We do that in the client module.

That's all you need since all the tests are identical for both implementations. In case we need specific tests for each implementation in the future, we can use the org.junit.jupiter.api.Tag, like we did here.

Our CI already handles both profiles. See, for example, the job for the reactive profile.

Tests are not exactly identical (the test class is). There are two different implementations in the Java projects one reactive and another non-reactive, and, if I merge them both into a single module, it will fail. I can make the tests use profiles to align with the rest of the project, but I don't think it's feasible to merge both modules.

Copy link
Contributor

@hbelmiro hbelmiro left a comment

Choose a reason for hiding this comment

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

@carlesarnal I see. I don't see a benefit in using profiles then.

@hbelmiro hbelmiro merged commit d0f8224 into quarkiverse:main Jun 13, 2024
11 checks passed
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.

Implement reactive support for server side code generation
3 participants