Skip to content

Switch to Java 17 for all Dataflow Templates#2050

Merged
liferoad merged 12 commits intomainfrom
liferoad-patch-1
Jan 15, 2025
Merged

Switch to Java 17 for all Dataflow Templates#2050
liferoad merged 12 commits intomainfrom
liferoad-patch-1

Conversation

@liferoad
Copy link
Copy Markdown
Contributor

@liferoad liferoad commented Dec 5, 2024 •
edited
Loading

Fixes #2051

@codecov
Copy link
Copy Markdown

codecov bot commented Dec 5, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.

Project coverage is 46.58%. Comparing base (9d39b07) to head (1bf3467).
Report is 10 commits behind head on main.

Files with missing lines Patch % Lines
...am/it/gcp/artifacts/matchers/ArtifactsSubject.java 0.00% 6 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff              @@
##               main    #2050      +/-   ##
============================================
- Coverage     46.58%   46.58%   -0.01%     
+ Complexity     3960     3950      -10     
============================================
  Files           869      867       -2     
  Lines         51599    51550      -49     
  Branches       5403     5398       -5     
============================================
- Hits          24036    24012      -24     
+ Misses        25844    25820      -24     
+ Partials       1719     1718       -1
Components Coverage Δ
spanner-templates 68.55% <ø> (-0.02%) ⬇️
spanner-import-export 65.57% <ø> (ø)
spanner-live-forward-migration 77.36% <ø> (ø)
spanner-live-reverse-replication 78.26% <ø> (-0.07%) ⬇️
spanner-bulk-migration 87.62% <ø> (ø)
Files with missing lines Coverage Δ
...gle/cloud/teleport/plugin/DockerfileGenerator.java 90.56% <ø> (ø)
...am/it/gcp/artifacts/matchers/ArtifactsSubject.java 0.00% <0.00%> (ø)

... and 7 files with indirect coverage changes

@Abacn
Copy link
Copy Markdown
Contributor

Abacn commented Dec 5, 2024

there is a single test failure from the report: https://github.com/GoogleCloudPlatform/DataflowTemplates/pull/2050/checks?check_run_id=33950822829

expected to contain hash: 81f7b7afd932b4754caaa9ba6ced7a8bcb2cbfec6857cf823e4d112125c6e939
but was                 : [org.apache.beam.it.gcp.artifacts.GcsArtifact@40673c72]
	at com.google.cloud.teleport.templates.BulkCompressorIT.baseCompress(BulkCompressorIT.java:85)
	at com.google.cloud.teleport.templates.BulkCompressorIT.testCompressGzip(BulkCompressorIT.java:55)

This is expected. The original assert was too strict, see

https://stackoverflow.com/questions/77305259/change-in-gzipoutputstream-behavior-in-java-17

@Abacn
Copy link
Copy Markdown
Contributor

Abacn commented Dec 5, 2024

Kafka PR running on https://github.com/GoogleCloudPlatform/DataflowTemplates/actions/runs/12183265029

Spanner PR running on https://github.com/GoogleCloudPlatform/DataflowTemplates/actions/runs/12183272085

@liferoad liferoad mentioned this pull request Dec 5, 2024
Closed
@liferoad liferoad added improvement Making existing code better and removed ignore-for-release labels Dec 13, 2024
@liferoad liferoad changed the title [DO NOT MERGE] Test Java 17 [DO NOT MERGE] Switch to Java 17 for all Dataflow Templates Dec 13, 2024
Polber
Polber suggested changes Dec 19, 2024
View reviewed changes
@liferoad
Copy link
Copy Markdown
Contributor Author

liferoad commented Dec 21, 2024

TODO: fix #2050 (comment)

@liferoad liferoad marked this pull request as ready for review January 9, 2025 14:54
@liferoad liferoad requested review from a team as code owners January 9, 2025 14:54
@liferoad liferoad changed the title [DO NOT MERGE] Switch to Java 17 for all Dataflow Templates Switch to Java 17 for all Dataflow Templates Jan 9, 2025
@liferoad liferoad requested a review from Abacn January 9, 2025 14:54
Abacn
Abacn approved these changes Jan 9, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@Abacn Abacn left a comment

Choose a reason for hiding this comment

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

Thanks! Just have a minor comment, could be follow up (so no need to push new commit that triggers all tests)

@liferoad
Copy link
Copy Markdown
Contributor Author

liferoad commented Jan 14, 2025

TODO: fix #2050 (comment)

Done.

@manitgupta
Copy link
Copy Markdown
Member

manitgupta commented Jan 15, 2025

Thanks, all Spanner PR are passing - https://github.com/GoogleCloudPlatform/DataflowTemplates/actions/runs/12183272085

manitgupta
manitgupta approved these changes Jan 15, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@manitgupta manitgupta left a comment

Choose a reason for hiding this comment

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

CC: @bharadwaj-aditya

@liferoad liferoad merged commit 5cda88d into main Jan 15, 2025
@liferoad liferoad deleted the liferoad-patch-1 branch January 15, 2025 14:43
@Abacn Abacn mentioned this pull request Jan 21, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@manitgupta manitgupta manitgupta approved these changes

@Abacn Abacn Abacn approved these changes

@asthamohta asthamohta Awaiting requested review from asthamohta asthamohta is a code owner automatically assigned from GoogleCloudPlatform/spanner-migrations-team

@VardhanThigle VardhanThigle Awaiting requested review from VardhanThigle VardhanThigle is a code owner automatically assigned from GoogleCloudPlatform/spanner-migrations-team

+1 more reviewer

@Polber Polber Polber requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

improvement Making existing code better size/L

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Feature Request]: Use Java 17 for Dataflow Templates

4 participants

@liferoad @Abacn @manitgupta @Polber