Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Mark Avro as provided in harness JAR #29412

Merged
merged 2 commits into from
Nov 13, 2023
Merged

Mark Avro as provided in harness JAR #29412

merged 2 commits into from
Nov 13, 2023

Conversation

bvolpato
Copy link
Contributor

@bvolpato bvolpato commented Nov 13, 2023 •
edited
Loading

Bug: #29413

After #27851, user code that depends on versions newer than Avro 1.8.2 are having problems running on Dataflow.

For example in https://github.com/GoogleCloudPlatform/DataflowTemplates, where we moved on to Avro 1.11.3, there were incompatibility errors:

Caused by: java.io.InvalidClassException: org.apache.avro.specific.SpecificRecordBase; local class incompatible: stream classdesc serialVersionUID = -1463700717714793795, local class serialVersionUID = 189988654766568477

and

Caused by: java.lang.NoSuchMethodError: 'boolean org.apache.avro.generic.GenericRecord.hasField(java.lang.String)' com.google.cloud.teleport.v2.transforms.FormatDatastreamRecordToJson.getMetadataIsDeleted(FormatDatastreamRecordToJson.java:258) com.google.cloud.teleport.v2.transforms.FormatDatastreamRecordToJson.apply(FormatDatastreamRecordToJson.java:123) com.google.cloud.teleport.v2.transforms.FormatDatastreamRecordToJson.apply(FormatDatastreamRecordToJson.java:51) org.apache.beam.sdk.extensions.avro.io.AvroSource$AvroBlock.readNextRecord(AvroSource.java:610)

The root cause is that Avro classes are now being shipped along with the /opt/apache/beam/jars/beam-sdks-java-harness.jar, which wasn't the case before.

Mark Avro as provided should solve the problem and allow users to control their Avro.

(Tried to relocate in #29407 but got some test failures, so trying this as an alternative)

@github-actions github-actions bot added the java label Nov 13, 2023
@bvolpato
Copy link
Contributor Author

Run Java PreCommit

@github-actions GitHub Actions
Copy link
Contributor

Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment assign set of reviewers

@bvolpato
Copy link
Contributor Author

Run Java PreCommit

@bvolpato bvolpato mentioned this pull request Nov 13, 2023
Closed
@bvolpato
Copy link
Contributor Author

@aromanenko-dev, can I get your thoughts here if including Avro on beam-sdks-java-harness.jar was on purpose?

@aromanenko-dev
Copy link
Contributor

aromanenko-dev commented Nov 13, 2023 •
edited
Loading

@bvolpato I guess it was added while working on removing Avro from "core" and some tests were failed because of this. If your PR fixes this - it's perfect!

@Abacn
Copy link
Contributor

Abacn commented Nov 13, 2023

java precommit currently fails unrelated issue on ":sdks:java:container:goBuild"

@damccorm
Copy link
Contributor

The precommit failure on Actions looks more real:

  • What went wrong:
    Execution failed for task ':sdks:java:harness:analyzeClassesDependencies'.

Dependency analysis found issues.
unusedDeclaredArtifacts

  • org.apache.avro:avro:1.8.2@jar

@damccorm
Copy link
Contributor

@bvolpato I think we need an extra permitUnusedDeclared at the bottom

@damccorm
Copy link
Contributor

I think it should be permitUnusedDeclared library.java.avro, but would be good to run the task locally to verify

damccorm
damccorm approved these changes Nov 13, 2023
View reviewed changes
Copy link
Contributor

@damccorm damccorm left a comment

Choose a reason for hiding this comment

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

LGTM - merging since the Jenkins issue is unrelated (due to the version of Go we have installed on our jenkins machines)

@damccorm damccorm merged commit abc0a05 into apache:master Nov 13, 2023
22 of 23 checks passed
damccorm pushed a commit that referenced this pull request Nov 13, 2023
@bvolpato @damccorm
Mark Avro as provided in harness JAR (#29412)
7435d9d 
* Mark Avro as provided in harness JAR

* Add permitUnusedDeclared for Avro
@damccorm damccorm mentioned this pull request Nov 13, 2023
Merged
3 tasks
damccorm added a commit that referenced this pull request Nov 13, 2023
@damccorm @bvolpato
Mark Avro as provided in harness JAR (#29412) (#29415)
55e7d19 
* Mark Avro as provided in harness JAR

* Add permitUnusedDeclared for Avro

Co-authored-by: Bruno Volpato <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@damccorm damccorm damccorm approved these changes

Assignees
No one assigned
Labels
java
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bvolpato @aromanenko-dev @Abacn @damccorm