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 Jackson DataConverter #401

Closed
mfateev opened this issue Oct 3, 2019 · 5 comments
Closed

Add Jackson DataConverter #401

mfateev opened this issue Oct 3, 2019 · 5 comments

Comments

@mfateev
Copy link
Contributor

mfateev commented Oct 3, 2019

Gson in the java client is printing this error when using Java 12:
WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by com.google.gson.internal.reflect.UnsafeReflectionAccessor (jar:file:/cloud.pco.orchestrator.jar!/BOOT-INF/lib/gson-2.8.5.jar!/) to constructor java.util.Optional()
WARNING: Please consider reporting this to the maintainers of com.google.gson.internal.reflect.UnsafeReflectionAccessor
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release
Looks like there is an issue for it here: google/gson#1540
But at the same time there hasn't been a release of Gson since over a year.

Consider switching the default data converter to Jackson.

@mhubbard
Copy link

Would like to +1 this as Jackson has built in support for Java 8 Time objects allowing you to send your java.time.Instant or java.time.ZonedDateTime as a string.

@RemcoBuddelmeijer
Copy link
Contributor

Would be a good thing indeed to be adding, especially when one day we want to be able to provide a Spring Boot starter for Cadence. +1

@RemcoBuddelmeijer
Copy link
Contributor

RemcoBuddelmeijer commented Jan 15, 2020

I suggest testing all changes done for this issue on both JDK 8 and JDK 13 so that both the current supported Java LTS version and the latest Java version have been covered.
What is your view on this? @mfateev

To reach this goal, I will be making a separate Docker and Docker-compose file that make this possible.

@mfateev
Copy link
Contributor Author

mfateev commented Jan 15, 2020

Should we use Multi-Release Jars to support multiple Java versions?

@RemcoBuddelmeijer
Copy link
Contributor

RemcoBuddelmeijer commented Jan 15, 2020

I think it would be a nice touch, however, I don't think it's something really for the Cadence Client, as it would require a lot more change and a lot more complications in the development cycle to implement this feature to it's fullest (versus having multiple builds of the client).
I do understand that this feature is more targeted towards (third-party) libraries and frameworks, which then makes this feature eligible for this Java Client; which then means that we would have to upgrade to either Java 9 or 11. (More on this in a later block)

When it comes to testing, I don't think that using it for the client, which is "bound" to a specific version, makes it easier to maintain the code. We will need to also maintain the tests which might have dependencies which are bound to a specific version, limiting us into even using these Java features. For example, we maybe want to use a new Java 13 feature but in order to test it, we might have to completely build a specific part of a testing framework our selves as these do not support these Java 13 features yet.

I do rather have it to support from a specific LTS version and to have sub-versions of the Cadence Java Client on other non-main (as in non-main Cadence supported) Java versions (kind of like development/POC versions of the Java client). I would suggest to instead of having the Multi-Release Jars, to support up to Java 11 (LTS) and to have versions that support certain features from Java 12, 13, etc.

@RemcoBuddelmeijer RemcoBuddelmeijer removed their assignment Mar 23, 2022
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

No branches or pull requests

4 participants