Update the protobuf dependency? #37
Comments
|
To make a case for this ticket, many libraries are now requiring |
chhabrakadabra
added a commit
to chhabrakadabra/feast
that referenced
this issue
Sep 30, 2022
- As of version `1.49`, the various python packages in the [grpc repo](https://github.com/grpc/grpc/tree/master/src/python) require `protobuf>=4.21.3`. Unfortunately, this is incompatible with all versions of `tensorflow-metadata` (see [this issue](tensorflow/metadata#37)). And since `piptools` doesn't backtrack during dependency resolution, the requirement files cannot be regenerated without adding an upper limit on these grpc libraries directly in `setup.py`. - The previous attempt to upgrade usages of the `mock_dynamodb2` decorator to the newest version failed. Since I'm not an expert in dynamodb, it made sense to just cap the test tool to the version already being used in CI. Signed-off-by: Abhin Chhabra <abhin.chhabra@shopify.com>
feast-ci-bot
pushed a commit
to feast-dev/feast
that referenced
this issue
Oct 5, 2022
* Initial implementation of BigTable online store. Signed-off-by: Abhin Chhabra <abhin.chhabra@shopify.com> * Attempt to run bigtable integration tests. Currently focusing on just getting the tests running locally. I've only build python3.8 requirements. Signed-off-by: Abhin Chhabra <abhin.chhabra@shopify.com> * Got the BigTable tests running in local containers Signed-off-by: Abhin Chhabra <chhabra.abhin@gmail.com> * Set serialization version when computing entity ID Signed-off-by: Abhin Chhabra <chhabra.abhin@gmail.com> * Switch to the recommended layout in bigtable. This was recommended by the BigTable dev team. Details of this layout will be added to the documentation in a future commit. Signed-off-by: Abhin Chhabra <chhabra.abhin@gmail.com> * Minor bugfixes. - If a row is empty when fetching data, don't process it more. - If a task in the threadpool fails, bubble up that failure. - If a `created_ts` is not available, use an empty string. `None` does not automatically serialize to bytes. Signed-off-by: Abhin Chhabra <chhabra.abhin@gmail.com> * Move BigTable online store out of contrib As per feedback on the PR. Signed-off-by: Abhin Chhabra <abhin.chhabra@shopify.com> * Attempt to run integration tests in CI. Provide the GCP project and the bigtable instance ID for the tests to connect to. Signed-off-by: Abhin Chhabra <abhin.chhabra@shopify.com> * Delete tables for entity-less feature views. Signed-off-by: Abhin Chhabra <abhin.chhabra@shopify.com> * Table names should be smaller than 50 characters This is BigTable's table length limit and it's causing test failures. Signed-off-by: Abhin Chhabra <abhin.chhabra@shopify.com> * Optimize bigtable reads. - Fetch all the rows in one bigtable fetch. - Get only the columns that are necessary (using a column regex filter). Signed-off-by: Abhin Chhabra <abhin.chhabra@shopify.com> * dynamodb: switch to `mock_dynamodb` The latest rebuilding of requirements has upgraded the `moto` library past the `4.0.0` release, which has a couple of breaking changes. Specifically, the `mock_dynamodb2` decorator has been deprecated. See https://github.com/spulec/moto/blob/master/CHANGELOG.md#400 for more details. The actual PR (getmoto/moto#4919) mentions that it's because the `mock_dynamodb` decorator is now equivalent to the `mock_dynamodb2` decorator. Signed-off-by: Abhin Chhabra <abhin.chhabra@shopify.com> * minor: rename `BigTable` to `Bigtable` This matches the GCP docs. Signed-off-by: Abhin Chhabra <abhin.chhabra@shopify.com> * Wrote some Bigtable documentation. Closely mirrors the docs for the other online stores. Signed-off-by: Abhin Chhabra <abhin.chhabra@shopify.com> * Bugfix: Deal with missing row keys. It looks like the bigtable client will just skip over non-existent row keys. Signed-off-by: Abhin Chhabra <abhin.chhabra@shopify.com> * Fix linting issues. Signed-off-by: Abhin Chhabra <abhin.chhabra@shopify.com> * Generate requirements files. - As of version `1.49`, the various python packages in the [grpc repo](https://github.com/grpc/grpc/tree/master/src/python) require `protobuf>=4.21.3`. Unfortunately, this is incompatible with all versions of `tensorflow-metadata` (see [this issue](tensorflow/metadata#37)). And since `piptools` doesn't backtrack during dependency resolution, the requirement files cannot be regenerated without adding an upper limit on these grpc libraries directly in `setup.py`. - The previous attempt to upgrade usages of the `mock_dynamodb2` decorator to the newest version failed. Since I'm not an expert in dynamodb, it made sense to just cap the test tool to the version already being used in CI. Signed-off-by: Abhin Chhabra <abhin.chhabra@shopify.com> * Don't bother materializing created timestamp. Had a discussion with Danny about whether it's useful to copy this column. He agreed that there's no value to storing this in the online store. Signed-off-by: Abhin Chhabra <abhin.chhabra@shopify.com> * Remove `tensorflow-metadata`. Turns out that this dependency is not required. We removed all references to it in [this PR](#2063), but did not remove it from `setup.py`. Removing it has caused many of the restrictions imposed in previous commits to be unnecessary. Signed-off-by: Abhin Chhabra <abhin.chhabra@shopify.com> * Minor fix to Bigtable documentation. Feedback from Danny mentioned that Bigtable should be able to store multiple versions of the same key and fetch the latest at read time. This makes sense and means that concurrent writes should work just fine. Signed-off-by: Abhin Chhabra <abhin.chhabra@shopify.com> * update roadmap docs Signed-off-by: Danny Chiao <danny@tecton.ai> * Fix roadmap doc Signed-off-by: Danny Chiao <danny@tecton.ai> * Change link to point to roadmap page Signed-off-by: Danny Chiao <danny@tecton.ai> * change order in roadmap Signed-off-by: Danny Chiao <danny@tecton.ai> Signed-off-by: Abhin Chhabra <abhin.chhabra@shopify.com> Signed-off-by: Abhin Chhabra <chhabra.abhin@gmail.com> Signed-off-by: Danny Chiao <danny@tecton.ai> Co-authored-by: Danny Chiao <danny@tecton.ai>
franciscojavierarceo
pushed a commit
to franciscojavierarceo/feast
that referenced
this issue
Oct 18, 2022
* Initial implementation of BigTable online store. Signed-off-by: Abhin Chhabra <abhin.chhabra@shopify.com> * Attempt to run bigtable integration tests. Currently focusing on just getting the tests running locally. I've only build python3.8 requirements. Signed-off-by: Abhin Chhabra <abhin.chhabra@shopify.com> * Got the BigTable tests running in local containers Signed-off-by: Abhin Chhabra <chhabra.abhin@gmail.com> * Set serialization version when computing entity ID Signed-off-by: Abhin Chhabra <chhabra.abhin@gmail.com> * Switch to the recommended layout in bigtable. This was recommended by the BigTable dev team. Details of this layout will be added to the documentation in a future commit. Signed-off-by: Abhin Chhabra <chhabra.abhin@gmail.com> * Minor bugfixes. - If a row is empty when fetching data, don't process it more. - If a task in the threadpool fails, bubble up that failure. - If a `created_ts` is not available, use an empty string. `None` does not automatically serialize to bytes. Signed-off-by: Abhin Chhabra <chhabra.abhin@gmail.com> * Move BigTable online store out of contrib As per feedback on the PR. Signed-off-by: Abhin Chhabra <abhin.chhabra@shopify.com> * Attempt to run integration tests in CI. Provide the GCP project and the bigtable instance ID for the tests to connect to. Signed-off-by: Abhin Chhabra <abhin.chhabra@shopify.com> * Delete tables for entity-less feature views. Signed-off-by: Abhin Chhabra <abhin.chhabra@shopify.com> * Table names should be smaller than 50 characters This is BigTable's table length limit and it's causing test failures. Signed-off-by: Abhin Chhabra <abhin.chhabra@shopify.com> * Optimize bigtable reads. - Fetch all the rows in one bigtable fetch. - Get only the columns that are necessary (using a column regex filter). Signed-off-by: Abhin Chhabra <abhin.chhabra@shopify.com> * dynamodb: switch to `mock_dynamodb` The latest rebuilding of requirements has upgraded the `moto` library past the `4.0.0` release, which has a couple of breaking changes. Specifically, the `mock_dynamodb2` decorator has been deprecated. See https://github.com/spulec/moto/blob/master/CHANGELOG.md#400 for more details. The actual PR (getmoto/moto#4919) mentions that it's because the `mock_dynamodb` decorator is now equivalent to the `mock_dynamodb2` decorator. Signed-off-by: Abhin Chhabra <abhin.chhabra@shopify.com> * minor: rename `BigTable` to `Bigtable` This matches the GCP docs. Signed-off-by: Abhin Chhabra <abhin.chhabra@shopify.com> * Wrote some Bigtable documentation. Closely mirrors the docs for the other online stores. Signed-off-by: Abhin Chhabra <abhin.chhabra@shopify.com> * Bugfix: Deal with missing row keys. It looks like the bigtable client will just skip over non-existent row keys. Signed-off-by: Abhin Chhabra <abhin.chhabra@shopify.com> * Fix linting issues. Signed-off-by: Abhin Chhabra <abhin.chhabra@shopify.com> * Generate requirements files. - As of version `1.49`, the various python packages in the [grpc repo](https://github.com/grpc/grpc/tree/master/src/python) require `protobuf>=4.21.3`. Unfortunately, this is incompatible with all versions of `tensorflow-metadata` (see [this issue](tensorflow/metadata#37)). And since `piptools` doesn't backtrack during dependency resolution, the requirement files cannot be regenerated without adding an upper limit on these grpc libraries directly in `setup.py`. - The previous attempt to upgrade usages of the `mock_dynamodb2` decorator to the newest version failed. Since I'm not an expert in dynamodb, it made sense to just cap the test tool to the version already being used in CI. Signed-off-by: Abhin Chhabra <abhin.chhabra@shopify.com> * Don't bother materializing created timestamp. Had a discussion with Danny about whether it's useful to copy this column. He agreed that there's no value to storing this in the online store. Signed-off-by: Abhin Chhabra <abhin.chhabra@shopify.com> * Remove `tensorflow-metadata`. Turns out that this dependency is not required. We removed all references to it in [this PR](feast-dev#2063), but did not remove it from `setup.py`. Removing it has caused many of the restrictions imposed in previous commits to be unnecessary. Signed-off-by: Abhin Chhabra <abhin.chhabra@shopify.com> * Minor fix to Bigtable documentation. Feedback from Danny mentioned that Bigtable should be able to store multiple versions of the same key and fetch the latest at read time. This makes sense and means that concurrent writes should work just fine. Signed-off-by: Abhin Chhabra <abhin.chhabra@shopify.com> * update roadmap docs Signed-off-by: Danny Chiao <danny@tecton.ai> * Fix roadmap doc Signed-off-by: Danny Chiao <danny@tecton.ai> * Change link to point to roadmap page Signed-off-by: Danny Chiao <danny@tecton.ai> * change order in roadmap Signed-off-by: Danny Chiao <danny@tecton.ai> Signed-off-by: Abhin Chhabra <abhin.chhabra@shopify.com> Signed-off-by: Abhin Chhabra <chhabra.abhin@gmail.com> Signed-off-by: Danny Chiao <danny@tecton.ai> Co-authored-by: Danny Chiao <danny@tecton.ai>
|
it looks like this is holding us back from upgrading to |
|
any word on this? |
|
any updates? |
|
@rtg0795 Would you mind taking a look at this? |
|
Any updates on this? EDIT: If you could publish any changes that loosen the Protobuf dependency as tensorflow-metadata==1.14.1 that would be appreciated. A 1.15.0 release with the loosened Protobuf dependency would be worthless without re-releasing all the Tensorflow libraries that depend on tensorflow-metadata because they're all scoped to tensorflow-metadata<1.15.0. |
|
@NeilGirdhar Hi. I see that 'protobuf>=3.20.3,<4.21' has been used for 1.14.0. Can you please explain or provide more details on why this is not sufficient? |
|
@rtg0795 I'll let others comment more on this, but if I remember, this was preventing upgrades to Python 3.12. See also: #37 (comment) and #37 (comment) |
|
@rtg0795 What are your thoughts? It's been almost two years since this was requested. |
|
@rtg0795 This is blocking us as well. |
|
Thank you for fixing this on master! For everyone watching this, we just need to wait for the next release or use the master version. Also, just noting that Protobuf 5 is now out 😄 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Could the protobuf dependency be incremented by two major versions please?
The text was updated successfully, but these errors were encountered: