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

Java SE support #150

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Java SE support #150

wants to merge 5 commits into from

Conversation

ikonkere
Copy link

  • SE server implementation with Weld SE CDI support;
  • updated BOM.

* SE server implementation with Weld SE CDI support;
* updated BOM.
@ikonkere ikonkere mentioned this pull request Aug 29, 2019
@MBJuric
Copy link
Member

MBJuric commented Aug 29, 2019

Thank you. We will have a look and let you know soon.

@MBJuric MBJuric requested a review from urbim August 29, 2019 10:02
Copy link
Member

@urbim urbim left a comment

Choose a reason for hiding this comment

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

Thanks for this contribution! I added some comments (mostly style).

It would be great if you could also create a simple sample application (as a PR to https://github.com/kumuluz/kumuluzee-samples) showcasing a simple injection and perhaps injection of some configuration properties from our configuration framework.

Thanks again!

se/server/pom.xml Outdated Show resolved Hide resolved
se/server/pom.xml Outdated Show resolved Hide resolved
se/server/pom.xml Outdated Show resolved Hide resolved
@ikonkere
Copy link
Author

ikonkere commented Sep 2, 2019

Created a PR with a sample: kumuluz/kumuluzee-samples#25

* Factored out all components unrelated to Weld from cdi-weld to new
project cdi-common;
* Created new project cdi-weld-se that incapsulates Weld SE support;
* Updated server-se with these changes;
* Updated parent and BOM pom.xml with new projects.
se/server/pom.xml Outdated Show resolved Hide resolved
se/server/pom.xml Outdated Show resolved Hide resolved
* Merged kumuluz-se-server to kumuluzee-cdi-weld-se and removed the
former.
* Removed corresponding projects from parent and BOM pom.xml
@ikonkere
Copy link
Author

Any progress on this?

@cen1
Copy link
Contributor

cen1 commented Apr 10, 2020

@ikonkere can you rebase to current?

I was testing this a bit and it makes sense as a weld-se wrapper. Unfortunately the extension ecosystem support is very rough. I managed to load the config extension, that was about it.

  • mp-config worked
  • log4j2 wanted a Servlet present
  • mp-rest-client wanted a JAX-RS implementation, unfortunately that one wants a Servlet.
  • I haven't tested kumuluzee-streaming but that would be a good candidate

It looks like we have some more work to do for enablement in extensions which are not web specific when this is merged.

@ikonkere
Copy link
Author

Well, obviously there's no servlet support under SE, so it doesn't make sense trying to start anything that depends on javax.servlet. If some extensions require it for some reason, then i suppose they must be run with kumuluzee-servlet family and not this one.

I'm surprised mp-rest-client didn't work because it does work for me with RESTEasy, but afair there was some tweaking involved. mp-reactive-messaging must also work, as i've specifically tested on it.

I will definitely have a look as soon as i'm out of isolation: unfortunately i have no useful working environment or remote access available right now.

@cen1
Copy link
Contributor

cen1 commented Apr 10, 2020

I think the rest-client failure is mainly due to no distinction between jax-rs client and server side dependencies in our packaged components. log4j2 I am not sure why, I think just an overreaching check in the extension code. I do not think it is necessary to have extensions ready before the merge, but it is something to keep in mind. Without this compatibility, the SE server has limited usage with not much else benefit compared to just using stock weld-se.

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.

4 participants