Skip to content

Mark Avro as provided in harness JAR#29412

Merged
damccorm merged 2 commits intoapache:masterfrom
bvolpato:provided-avro
Nov 13, 2023
Merged

Mark Avro as provided in harness JAR#29412
damccorm merged 2 commits intoapache:masterfrom
bvolpato:provided-avro

Conversation

@bvolpato
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

bvolpato commented Nov 13, 2023

Run Java PreCommit

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Nov 13, 2023

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
Copy Markdown
Contributor Author

bvolpato commented Nov 13, 2023

Run Java PreCommit

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

bvolpato commented Nov 13, 2023

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

@aromanenko-dev
Copy link
Copy Markdown
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
Copy Markdown
Contributor

Abacn commented Nov 13, 2023

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

@damccorm
Copy link
Copy Markdown
Contributor

damccorm commented Nov 13, 2023

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
Copy Markdown
Contributor

damccorm commented Nov 13, 2023

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

@damccorm
Copy link
Copy Markdown
Contributor

damccorm commented Nov 13, 2023

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
Copy Markdown
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
damccorm pushed a commit that referenced this pull request Nov 13, 2023
@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
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 <bvolpato@google.com>
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