Skip to content

Fix Release Nightly Snapshot job - #40685

Merged
Abacn merged 7 commits into
masterfrom
fix-jms-compile
Oct 20, 2026
Merged

Abacn merged 7 commits into
masterfrom
fix-jms-compile

Conversation

@Amar3tto

Copy link
Copy Markdown
Collaborator

@Amar3tto
Amar3tto requested a review from damccorm October 20, 2026 13:05

@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 resolves compiler/ErrorProne errors in the Release Nightly Snapshot workflow by removing the deprecated finalize() method from JmsIO.UnboundedJmsReader (which implements Closeable via UnboundedReader and is managed by the runner) and adding @SuppressWarnings("PatternMatchingInstanceof") to DeltaWriteTestUtils and AddFilesIT to maintain Java 8 compatibility.

🔍 General Feedback

  • The changes are minimal, targeted, and cleanly address the JDK 17+ / ErrorProne build failures while preserving Java 8 source compatibility.

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

@SuppressWarnings({"Finalize", "removal"})
protected void finalize() {
doClose();
}

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.

@Abacn could you take a look at this piece?

@Amar3tto could you add context on this piece?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The finalize() method is deprecated and marked for removal in newer Java versions. Since UnboundedJmsReader already implements close() to release JMS resources, the finalizer is redundant. Resource cleanup should be handled explicitly through the reader lifecycle rather than relying on GC, which does not guarantee finalizer execution.

@github-actions

Copy link
Copy Markdown
Contributor

Assigning reviewers:

R: @ahmedabu98 for label iceberg.
Backup reviewer: chamikaramj (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).

@Abacn Abacn 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.

Since there are multiple occurrence, we should add PatternMatchingInstanceOf into the global errorprome exclusion list, in BeamModulePlugin

@github-actions github-actions Bot added the build label Oct 20, 2026
@github-actions github-actions Bot removed delta iceberg Iceberg IO connector labels Oct 20, 2026
@Abacn
Abacn merged commit 7bdc6ff into master Oct 20, 2026
28 checks passed
@Abacn
Abacn deleted the fix-jms-compile branch October 20, 2026 18:38
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.

The Release Nightly Snapshot job is flaky

3 participants