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

GlassFish support #282

Merged

Conversation

OndroMih
Copy link
Contributor

An alternative to #273 with a connector from the Arquillian project.

However, there are a few drawbacks:

  • There's no GlassFish Embedded connector from the Arquillian project for GlassFish 7.x yet (as stated in https://github.com/arquillian/arquillian-container-glassfish6#readme: "the embedded container support still needs to be updated for GlassFish 6.x")
  • This setup uses GlassFish managed connector, which is slower to start and harder to configure in the project.
    • It downloads GlassFish from Maven Central and installs it using the dependency Maven plugin.
    • To modify the HTTP port from 8080 to 9090, a config file needs to be modified, this is done with an Antrun plugin script.
    • The Maven surefire plugin needs to be configured to set the path to the GlassFish installation for the Arquillian connector. The configuration in arquillian.xml is also more complicated than with the Embedded connector
  • The latest released version of this connector is 1.0.0.Alpha1 and it's still not stable and safe to use
    • It depends on a milestone version of the jersey-hk2 (3.0.0-M4), which depends on an artifact, which isn't in Maven Central (hk2-locator version 3.0.0-M1). I added an explicit dependency on jersey-hk2 version 3.1.3 to fix the build
    • the name of the arquillian connector artifact has changed in the Arquillian source code repository from arquillian-glassfish-managed-6 to arquillian-glassfish-managed-jakarta, although the latter hasn't been released to Maven Central yet: https://github.com/arquillian/arquillian-container-glassfish6/blob/master/glassfish-managed/pom.xml#L12

Signed-off-by:Ondro Mihalyi <mihalyi@omnifish.ee>
Signed-off-by:Ondro Mihalyi <mihalyi@omnifish.ee>
Conflicts:
	pom.xml

Signed-off-by:Ondro Mihalyi <mihalyi@omnifish.ee>
Use GlassFish Embedded container.
Signed-off-by:Ondro Mihalyi <mihalyi@omnifish.ee>
Some XML parsers like Xerces don't follow the HTTP to HTTPS redirect.
Signed-off-by:Ondro Mihalyi <mihalyi@omnifish.ee>
Signed-off-by:Ondro Mihalyi <mihalyi@omnifish.ee>
Signed-off-by:Ondro Mihalyi <mihalyi@omnifish.ee>
Signed-off-by:Ondro Mihalyi <mihalyi@omnifish.ee>
Signed-off-by:Ondro Mihalyi <mihalyi@omnifish.ee>
@m-reza-rahman
Copy link
Contributor

These look like a reasonable set of changes I could move forward to merge. What is causing the draft status? Are the changes ready for me to test or is there some other work left to finalize the PR?

@OndroMih
Copy link
Contributor Author

These look like a reasonable set of changes I could move forward to merge. What is causing the draft status? Are the changes ready for me to test or is there some other work left to finalize the PR?

I raised the PR in draft because I prefer to merge #273 because of the reasons stated in the description.

But since you suggested to use this PR instead, I'll remove the draft status now and close #273

Signed-off-by:Ondro Mihalyi <mihalyi@omnifish.ee>
@OndroMih OndroMih marked this pull request as ready for review August 28, 2023 23:28
@m-reza-rahman m-reza-rahman changed the title Glassfish7 Maven profile with Arquillian project's GlassFish Managed connector GlassFish 7 Maven profile (with Arquillian GlassFish managed connector) Aug 28, 2023
@m-reza-rahman m-reza-rahman changed the title GlassFish 7 Maven profile (with Arquillian GlassFish managed connector) GlassFish 7 Maven profile Aug 28, 2023
@m-reza-rahman
Copy link
Contributor

I'll look to test and merge this. There may be some delay as I am currently on vacation. Hopefully there is no hurry?

@OndroMih
Copy link
Contributor Author

There's no hurry at all. Take your time and enjoy your vacation!

@m-reza-rahman
Copy link
Contributor

I've briefly tested this and it looks fine. @ojuschugh1 will also give it a spin. Then it can be merged.

@ojuschugh1
Copy link
Contributor

@m-reza-rahman I have tested it via command line and vs code(debugged and tested) for Glassfish and Payara. It is working fine. I have tested the batch job as well, it is also working properly. A total of 29/29 test cases passed without any error.

@m-reza-rahman
Copy link
Contributor

Great. I’ll merge this as soon I have bandwidth to update all the associated documentation on the wiki and website.

@m-reza-rahman m-reza-rahman changed the title GlassFish 7 Maven profile GlassFish support Sep 17, 2023
@m-reza-rahman m-reza-rahman merged commit fa925bd into eclipse-ee4j:master Sep 17, 2023
1 check passed
@m-reza-rahman m-reza-rahman mentioned this pull request Sep 17, 2023
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