Skip to content

[BEAM-5624] Fix avro.schema parser for py3#6616

Merged
aaltay merged 1 commit into
apache:masterfrom
splovyt:avro-parse
Oct 11, 2018
Merged

[BEAM-5624] Fix avro.schema parser for py3#6616
aaltay merged 1 commit into
apache:masterfrom
splovyt:avro-parse

Conversation

@splovyt
Copy link
Copy Markdown
Contributor

@splovyt splovyt commented Oct 9, 2018

Fix for the following error mentioned in BEAM-5624:
AttributeError (module 'avro.schema' has no attribute 'parse')

This is is part of a series of PRs with goal to make Apache Beam PY3 compatible. The proposal with the outlined approach has been documented here.

@tvalentyn @Fematich @charlesccychen @aaltay @Juta @manuzhang

Post-Commit Tests Status (on master branch)

Lang SDK Apex Dataflow Flink Gearpump Samza Spark
Go Build Status --- --- --- --- --- ---
Java Build Status Build Status Build Status Build Status Build Status Build Status Build Status
Python Build Status --- Build Status
Build Status
Build Status --- --- ---

import hamcrest as hc
# pylint: disable=wrong-import-order, wrong-import-position, ungrouped-imports
try:
from avro.schema import Parse
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you add a comment here about, in what versions of avro which version of parse is supported. (We can use this information to remove this block later on.)

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.

+1 to @aaltay's comment. Also we will need a similar change in other places in Beam where we use avro.schema.parse. See apache_beam/io/avroio.py, apache_beam/examples/avro_bitcoin.py. This can be done in another PR if you prefer.

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.

also apache_beam/examples/fastavro_it_test.py.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thank you both for the feedback.

@aaltay I have added a comment stating that python2 uses the 'avro' library, whereas python3 uses 'avro-python3'.
@tvalentyn I have made the changes in the related files as well.

Note that although I had disabled pylint wrong-import-order and wrong-import-position, it still did not pass the precommit check so I had to reposition the imports to continue, PTAL, thanks!

@splovyt splovyt force-pushed the avro-parse branch 8 times, most recently from 34225ec to e2c25d5 Compare October 11, 2018 12:18
Copy link
Copy Markdown
Contributor

@tvalentyn tvalentyn left a comment

Choose a reason for hiding this comment

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

Thank you!

@aaltay aaltay merged commit 4774630 into apache:master Oct 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants