tvalentyn commented on a change in pull request #15900:
URL: https://github.com/apache/beam/pull/15900#discussion_r743886861
##########
File path: sdks/python/apache_beam/examples/fastavro_it_test.py
##########
@@ -124,23 +123,22 @@ def batch_indices(start):
| 'create-records' >> Map(record)
fastavro_output = '/'.join([self.output, 'fastavro'])
- avro_output = '/'.join([self.output, 'avro'])
+ # avro_output = '/'.join([self.output, 'avro'])
# pylint: disable=expression-not-assigned
records_pcoll \
| 'write_fastavro' >> WriteToAvro(
fastavro_output,
parse_schema(json.loads(self.SCHEMA_STRING)),
- use_fastavro=True
)
- # pylint: disable=expression-not-assigned
- records_pcoll \
- | 'write_avro' >> WriteToAvro(
- avro_output,
- Parse(self.SCHEMA_STRING),
- use_fastavro=False
- )
+ # # pylint: disable=expression-not-assigned
Review comment:
We do not keep commented-out code - we can just remove this.
If we ever need this code again, we can look it up in revision history.
##########
File path: sdks/python/apache_beam/io/avroio.py
##########
@@ -510,8 +301,6 @@ def __init__(
is '-SSSSS-of-NNNNN' if None is passed as the shard_name_template.
mime_type: The MIME type to use for the produced files, if the filesystem
supports specifying MIME types.
- use_fastavro (bool); when set, use the `fastavro` library for IO, which
Review comment:
We can still remove the flag from internal code that is not subject to
backwards-compatibility.
##########
File path: sdks/python/apache_beam/io/avroio.py
##########
@@ -510,8 +301,6 @@ def __init__(
is '-SSSSS-of-NNNNN' if None is passed as the shard_name_template.
mime_type: The MIME type to use for the produced files, if the filesystem
supports specifying MIME types.
- use_fastavro (bool); when set, use the `fastavro` library for IO, which
Review comment:
As a courtesy to users, let's keep this flag in user-facing public APIs,
but make it have no effect.
Otherwise, when users update to a new SDK, their pipeline will start to fail
at startup and will require changes. This is disruptive. Instead, let's say in
docstrings (where applicable):
```use_fastavro: This flag is left for API backwards compatibility and no
longer has an effect. Do not use. ```
##########
File path: sdks/python/setup.py
##########
@@ -177,6 +176,7 @@ def get_version():
'sqlalchemy>=1.3,<2.0',
'psycopg2-binary>=2.8.5,<3.0.0',
'testcontainers>=3.0.3,<4.0.0',
+ 'avro-python3>=1.8.1,!=1.9.2,<1.10.0',
Review comment:
We should not need to depend on this package anymore.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
