Skip to content

add snakeyaml dep to managed module - #40981

Merged
ahmedabu98 merged 1 commit into
masterfrom
core-snakeyaml-pom
Nov 4, 2026
Merged

ahmedabu98 merged 1 commit into
masterfrom
core-snakeyaml-pom

Conversation

@ahmedabu98

Copy link
Copy Markdown
Contributor

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

@github-actions github-actions Bot added java io delta size/xs XS: <10 lines changed labels Nov 4, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📋 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.gradle properly pairs implementation library.java.snake_yaml with permitUnusedDeclared library.java.snake_yaml to satisfy the analyzeClassesDependencies task while ensuring transitive runtime availability for consumers of :sdks:java:managed.
  • Removing the hardcoded org.yaml:snakeyaml:2.0 test runtime dependencies in sdks/java/io/delta/build.gradle and sdks/java/managed/build.gradle aligns SnakeYAML versioning with the centralized library.java.snake_yaml catalog entry in BeamModulePlugin.

💡 Tip: Comment /gemini review on this pull request to trigger a new Gemini code review.

@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Assigning reviewers:

R: @chamikaramj for label delta.
Backup reviewer: Abacn (if the main reviewer is unresponsive, you can escalate to them by tagging their username)

Note: If you would like to opt out of this review, comment assign to next reviewer.

Available commands:

  • stop reviewer notifications - opt out of the automated review tooling
  • remind me after tests pass - tag the comment author after tests pass
  • waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

The PR bot will only process comments in the main thread (not review comments).

Comment on lines +34 to +37
// 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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

java core does not include snakeyaml in its published shaded jar

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not sure why this was added but as long as we are not seeing failures, we could remove.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nvm, see that you are adding here.

@chamikaramj chamikaramj left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks!

Comment on lines +34 to +37
// 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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not sure why this was added but as long as we are not seeing failures, we could remove.

@ahmedabu98
ahmedabu98 merged commit 12547d4 into master Nov 4, 2026
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants