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

[BEAM-394] Updated the AvroCoder class to set unserializable fields as transient #624

Closed
wants to merge 3 commits into from
Closed

[BEAM-394] Updated the AvroCoder class to set unserializable fields as transient #624

wants to merge 3 commits into from

Conversation

ilganeli
Copy link

  • Updated AvroCoder to mark non-serializable fields as transient.
  • Updated the findbugs_filter to remove ignored errors.

Updated the AvroCoder class to set unserializable fields as transient…
131b425 
…. Updated the findbugs_filter to remove ignored errors.
@dhalperi
Copy link
Contributor

Hi Ilya,

Thanks, this is great!

A few Checkstyle errors are breaking the build:

[ERROR] /Users/travis/build/apache/incubator-beam/sdks/java/core/src/main/java/org/apache/beam/sdk/coders/AvroCoder.java:171:21: 'final' modifier out of order with the JLS suggestions. [ModifierOrder]

[ERROR] /Users/travis/build/apache/incubator-beam/sdks/java/core/src/main/java/org/apache/beam/sdk/coders/AvroCoder.java:181:21: 'final' modifier out of order with the JLS suggestions. [ModifierOrder]

[ERROR] /Users/travis/build/apache/incubator-beam/sdks/java/core/src/main/java/org/apache/beam/sdk/coders/AvroCoder.java:182:21: 'final' modifier out of order with the JLS suggestions. [ModifierOrder]

[ERROR] /Users/travis/build/apache/incubator-beam/sdks/java/core/src/main/java/org/apache/beam/sdk/coders/AvroCoder.java:183:21: 'final' modifier out of order with the JLS suggestions. [ModifierOrder]

[ERROR] /Users/travis/build/apache/incubator-beam/sdks/java/core/src/main/java/org/apache/beam/sdk/coders/AvroCoder.java:184:21: 'final' modifier out of order with the JLS suggestions. [ModifierOrder]

[ERROR] /Users/travis/build/apache/incubator-beam/sdks/java/core/src/test/java/org/apache/beam/sdk/coders/AvroCoderTest.java:65:8: Unused import: java.io.IOException. [UnusedImports]

[ERROR] /Users/travis/build/apache/incubator-beam/sdks/java/core/src/test/java/org/apache/beam/sdk/coders/AvroCoderTest.java:68:8: Unused import: java.io.OutputStream. [UnusedImports]

[ERROR] /Users/travis/build/apache/incubator-beam/sdks/java/core/src/test/java/org/apache/beam/sdk/coders/AvroCoderTest.java:155: First sentence should end with a period. [JavadocStyle]

[ERROR] /Users/travis/build/apache/incubator-beam/sdks/java/core/src/test/java/org/apache/beam/sdk/coders/AvroCoderTest.java:156: Line is longer than 100 characters (found 111). [LineLength]

[ERROR] /Users/travis/build/apache/incubator-beam/sdks/java/core/src/test/java/org/apache/beam/sdk/coders/AvroCoderTest.java:162:3: '{' at column 3 should be on the previous line. [LeftCurly]

Audit done.

[INFO] There are 10 errors reported by Checkstyle 6.17 with beam/checkstyle.xml ruleset.

@ilganeli
Copy link
Author

ilganeli commented Jul 11, 2016 •
edited

@dhalperi Thanks! Is there a checkstyle and code style config somewhere I can import into IntelliJ?

@dhalperi
Copy link
Contributor

Checkstyle config is in the build-tools somewhere -- should be easy to find. (Sorry, running off to a meeting). checkstyle.xml?

dhalperi
dhalperi reviewed Jul 11, 2016
View reviewed changes
sdks/java/core/src/main/java/org/apache/beam/sdk/coders/AvroCoder.java
private final ThreadLocal<BinaryEncoder> encoder;
private final ThreadLocal<DatumWriter<T>> writer;
private final ThreadLocal<DatumReader<T>> reader;
private final transient ThreadLocal<BinaryDecoder> decoder;
Copy link
Contributor

Choose a reason for hiding this comment

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

please remove extra space.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, I can do this in merge.

@dhalperi
Copy link
Contributor

LGTM -- if you can fix the extra space great, or I can fix during the merge.

@dhalperi
Copy link
Contributor

R: @dhalperi

Still LGTM, will merge when tests are green.

@dhalperi
Copy link
Contributor

Seems like Travis was a flake, still merging.

@asfgit asfgit closed this in d93f564 Jul 12, 2016
johnjcasey pushed a commit to johnjcasey/beam that referenced this pull request Feb 8, 2023
@aaltay
Merge pull request apache#624 from TheNeuralBit/current-2.37.0
b2b3918
pl04351820 pushed a commit to pl04351820/beam that referenced this pull request Dec 20, 2023
@parthea
fix(deps): allow protobuf < 5.0.0 (apache#624)
4baf837 
fix(deps): require proto-plus >= 1.22.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@ilganeli @dhalperi