search

awslabs / kinesis-aggregation

AWS libraries/modules for working with Kinesis aggregated record data
Apache License 2.0
376 stars 154 forks source link

Fix bundled protobuf files #161

Closed maxispeicher closed 2 years ago

maxispeicher commented 2 years ago

Description of changes: The currently versions 1.2.1 and 1.2.2 of the released python package can not be used as the required protobuf files are not included in the package anymore. E.g. running 

from aws_kinesis_agg.aggregator import RecordAggregator

results in

ModuleNotFoundError: No module named 'messages_pb2'

.

This PR reincludes the required protobuf files in the package. Additionally it would probably be a good idea to yank 1.2.1 and 1.2.2.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

IanMeyers commented 2 years ago

Unfortunately this was the package structure from 1.2.1, which has an issue with import paths in generated files. Version 1.2.2 should address this correctly now, with generated protobuf files included from the project root.

IanMeyers commented 2 years ago

I can see messages_pb2.py and config_pb2.py included in the 1.2.2 tar.

IanMeyers commented 2 years ago

I can repro - looking into it

maxispeicher commented 2 years ago

By having https://github.com/awslabs/kinesis-aggregation/blob/master/python/setup.py#L25 you basically tell that only the aws_kinesis_agg directory should be included when installing. You could also add something like py_modules=['messages_pb2', "config_pb2"], to the setup call and thereby the modules would be included. However, this inclusion would be directly in site-packages and thereby there might be a risk of name clashes as they are not in the aws_kinesis_agg package.

IanMeyers commented 2 years ago

These files are included through the MANIFEST.in file in 1.2.2. Trying to figure out root cause of import error. Moving these generated files to aws_kinesis_agg package isn't an option because they then throw path errors.

maxispeicher commented 2 years ago

AFAIK this only includes in the source distribution but not in the package that is installed.

Do you know which path errors occur when including these files? For me it works like this.

IanMeyers commented 2 years ago

Please update to version 1.2.3. The protobuf generated files have been re-sited as in this PR, but with build artefacts in place. If this still doesn't work, please re-open.

maxispeicher commented 2 years ago

Version 1.2.3 seems to work again. Thanks :+1: