-
Notifications
You must be signed in to change notification settings - Fork 564
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
[3.x] Adapt to SnakeYAML 2.0 changes #5793
[3.x] Adapt to SnakeYAML 2.0 changes #5793
Conversation
config/yaml-mp/src/main/java/io/helidon/config/yaml/mp/YamlMpConfigSource.java
Show resolved
Hide resolved
e65abcd
to
6773fc8
Compare
@@ -111,6 +111,7 @@ static class CustomRepresenter extends Representer { | |||
CustomRepresenter(Map<Class<?>, ExpandedTypeDescription> types, | |||
Map<Class<?>, ExpandedTypeDescription> implsToTypes, DumperOptions dumperOptions, | |||
DumperOptions.ScalarStyle stringStyle) { | |||
super(dumperOptions); | |||
this.implsToTypes = implsToTypes; | |||
this.dumperOptions = dumperOptions; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Do you still need this?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. The no-args constructor in the superclass was removed. In fact, the private field is never used and I need to remove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, that's what I meant (the private
field).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to be missing the upgrade to SnakeYaml 2.0?
I was able to build/run the test app with SnakeYaml 2 and it passes the unit tests (with verifySnakeYamlVersion
disabled). But when I run it by hand it reports a pile of errors like these:
2023.03.07 13:00:46 WARNING org.yaml.snakeyaml.introspector Thread[main,5,main]: Failed to find field for org.eclipse.microprofile.openapi.models.media.Schema.enum
2023.03.07 13:00:46 WARNING org.yaml.snakeyaml.introspector Thread[main,5,main]: Failed to find [getExtensions(0 args)] for org.eclipse.microprofile.openapi.models.media.XML.extensions
I accidentally omitted the change to I've reproduced the warnings and am investigating. |
The warnings are appearing now from the SnakeYAML Similar warnings now appear when, for example, building our QuickStart app (which has OpenAPI support in it). The good news is that the internal model is being built correctly, despite these warnings, but the messages are alarming. I'm in the process of looking for exactly why these messages are appearing despite the model being built properly. Once we know that, we can look for a way to avoid the messages. |
I have found the immediate cause of the messages. It's not yet clear what the best workaround will be. See https://groups.google.com/g/snakeyaml-core/c/JOzKbNLorHs/m/K2XO2on7AgAJ where I explained the cause and have asked about a solution. |
The MP OpenAPI TCK fails because it uses Jackson dataformat which in turn uses SnakeYAML, in particular at least one 1.x constructor removed in 2.0:
Jackson has updated to use SnakeYAML 2.0 (FasterXML/jackson-dataformats-text#390) but has not yet issued a release containing the fix. It seems targeted for 2.15 which is forecast to have a release candidate in March 2023. https://github.com/FasterXML/jackson/wiki/Jackson-Release-2.15 In the meantime, we can override which release the TCK uses while still using SnakeYAML 2.0 to build our artifacts. |
…to verify updated Helidon code works with the earlier version
The mapping should be the same. SnakeYaml 2 is now an MR jar with a module-info and a custom logging implementation on java 9+ (switches from JUL to the java platform logger). I based the change off of FINE (in JUL) having a severity of 500 and DEBUG (in JPL) having a severity of 500. |
Resolves #5792
SnakeYAML 2.0 is now released.
This PR updates Helidon's own code to work with either 2.0 or 1.32.
It includes changes to config and OpenAPI, as well as an integration test verifying that config and OpenAPI work with 1.32.
A change in SnakeYAML from 1.32 to 2.0 now logs many
WARNING
messages (they wereFINE
in 1.32). To my mind, these are due to a shortcoming in SnakeYAML, not in how our code uses it. I have asked about this in the Google group for SnakeYAML but have not heard back yet. In the meantime, I've included code in this PR to change the logging level for the relevant SnakeYAML logger toSEVERE
and added a config setting with which a user could disable our warning suppression to see those and other more important warnings if necessary.Most of the other changes are so our code uses more secure APIs in SnakeYAML and works with either 1.32 or 2.0.