Repository navigation
add snakeyaml dep to managed module - #40981
Conversation
There was a problem hiding this comment.
📋 Review Summary
This PR adds library.java.snake_yaml as an implementation dependency in :sdks:java:managed (with permitUnusedDeclared for Gradle dependency analysis) and removes the redundant testRuntimeOnly workaround from :sdks:java:io:delta and :sdks:java:managed. Because :sdks:java:core shades and relocates SnakeYAML in its shadow configuration, downstream modules consuming Managed.from(...).withConfigUrl(...) need org.yaml.snakeyaml on their runtime classpath when YamlUtils invokes YamlReader.
🔍 General Feedback
- The dependency configuration in
sdks/java/managed/build.gradleproperly pairsimplementation library.java.snake_yamlwithpermitUnusedDeclared library.java.snake_yamlto satisfy theanalyzeClassesDependenciestask while ensuring transitive runtime availability for consumers of:sdks:java:managed. - Removing the hardcoded
org.yaml:snakeyaml:2.0test runtime dependencies insdks/java/io/delta/build.gradleandsdks/java/managed/build.gradlealigns SnakeYAML versioning with the centralizedlibrary.java.snake_yamlcatalog entry inBeamModulePlugin.
💡 Tip: Comment /gemini review on this pull request to trigger a new Gemini code review.
|
Assigning reviewers: R: @chamikaramj for label delta. Note: If you would like to opt out of this review, comment Available commands:
The PR bot will only process comments in the main thread (not review comments). |
| // Core's YamlUtils needs snakeyaml at runtime, which is used here and in managed IOs | ||
| // downstream. | ||
| implementation library.java.snake_yaml | ||
| permitUnusedDeclared library.java.snake_yaml |
There was a problem hiding this comment.
java core does not include snakeyaml in its published shaded jar
There was a problem hiding this comment.
Not sure why this was added but as long as we are not seeing failures, we could remove.
There was a problem hiding this comment.
Nvm, see that you are adding here.
| // Core's YamlUtils needs snakeyaml at runtime, which is used here and in managed IOs | ||
| // downstream. | ||
| implementation library.java.snake_yaml | ||
| permitUnusedDeclared library.java.snake_yaml |
There was a problem hiding this comment.
Not sure why this was added but as long as we are not seeing failures, we could remove.
Add snakeyaml to Managed module. Managed transforms use YamlUtils (from core) but core doesn't publish snakeyaml in its POM, so Managed transforms fail with NoClassDefFoundError unless the user explicitly includes snakeyaml.
This adds it to the Managed module to avoid the need to do that