Skip to content

chore: Unpin googleapis-common-protos#2745

Merged
feast-ci-bot merged 4 commits intofeast-dev:masterfrom
chhabrakadabra:unpin-googleapis-common-protos
Jun 2, 2022
Merged

chore: Unpin googleapis-common-protos#2745
feast-ci-bot merged 4 commits intofeast-dev:masterfrom
chhabrakadabra:unpin-googleapis-common-protos

Conversation

@chhabrakadabra
Copy link
Collaborator

@chhabrakadabra chhabrakadabra commented May 30, 2022

What this PR does / why we need it:

Exact pinning this library can cause Feast to be incompatible with other libraries that users may want to install.

Which issue(s) this PR fixes:

Fixes #

@chhabrakadabra chhabrakadabra changed the title Unpin googleapis-common-protos chore: Unpin googleapis-common-protos May 30, 2022
@codecov-commenter
Copy link

codecov-commenter commented May 30, 2022 •
edited
Loading

Codecov Report

Merging #2745 (4779176) into master (de97bd3) will decrease coverage by 0.04%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #2745      +/-   ##
==========================================
- Coverage   80.28%   80.24%   -0.05%     
==========================================
  Files         169      169              
  Lines       14366    14366              
==========================================
- Hits        11534    11528       -6     
- Misses       2832     2838       +6
Flag Coverage Δ
integrationtests 70.45% <0.00%> (-0.38%) ⬇️
unittests 58.73% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
setup.py 0.00% <0.00%> (ø)
sdk/python/tests/utils/online_read_write_test.py 93.54% <0.00%> (-6.46%) ⬇️
.../integration/online_store/test_online_retrieval.py 96.84% <0.00%> (-3.16%) ⬇️
...ython/feast/embedded_go/online_features_service.py 89.14% <0.00%> (-0.78%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update de97bd3...4779176. Read the comment docs.

@chhabrakadabra chhabrakadabra marked this pull request as ready for review May 30, 2022 20:40
@achals
Copy link
Member

achals commented May 30, 2022

You'll probably need to regenerate the requirements files!

@chhabrakadabra
Copy link
Collaborator Author

chhabrakadabra commented May 31, 2022

You'll probably need to regenerate the requirements files!

Whoops! Thanks.

@chhabrakadabra
Copy link
Collaborator Author

chhabrakadabra commented Jun 1, 2022

I had some trouble with generating the requirements files and I would really appreciate it if someone could double-check my work.

For one, the make targets lock-python-dependencies and lock-python-ci-dependencies work differently from the commands right at the top (in the comments) in the requirements files and produce very different results. For example, the command PYTHON=3.7 make lock-python-ci-dependencies fails to generate the 3.7 CI requirements file, but the command at the top of that file pip-compile --output-file=sdk/python/requirements/py3.7-requirements.txt succeeds.

Another issue is why that make target mentioned above fails. It fails with the following error:

(.venv37) abhin@Abhins-personal-MB feast % PYTHON=3.7 make lock-python-ci-dependencies                             
python -m piptools compile -U --extra ci --output-file sdk/python/requirements/py3.7-ci-requirements.txt
ERROR:pep517.envbuild:ERROR: pip's dependency resolver does not currently take into account all the packages that are installed. This behaviour is the source of the following dependency conflicts.
ERROR:pep517.envbuild:sphinx-rtd-theme 1.0.0 requires docutils<0.18, but you have docutils 0.18.1 which is incompatible.
ERROR:pep517.envbuild:great-expectations 0.14.13 requires jinja2<3.1.0,>=2.10, but you have jinja2 3.1.2 which is incompatible.
ERROR:pep517.envbuild:great-expectations 0.14.13 requires pyparsing<3,>=2.4, but you have pyparsing 3.0.9 which is incompatible.
ERROR:pep517.envbuild:flake8 4.0.1 requires importlib-metadata<4.3; python_version < "3.8", but you have importlib-metadata 4.11.4 which is incompatible.
ERROR:pep517.envbuild:feast 0.21.1.dev31+g46006d2e.d20220531 requires protobuf<3.20,>=3.10, but you have protobuf 3.20.1 which is incompatible.
ERROR:pep517.envbuild:WARNING: You are using pip version 22.0; however, version 22.1.2 is available.
ERROR:pep517.envbuild:You should consider upgrading via the '/Users/abhin/src/github.com/chhabrakadabra/feast/.venv37/bin/python -m pip install --upgrade pip' command.
ERROR:pep517.envbuild:ERROR: pip's dependency resolver does not currently take into account all the packages that are installed. This behaviour is the source of the following dependency conflicts.
ERROR:pep517.envbuild:sphinx-rtd-theme 1.0.0 requires docutils<0.18, but you have docutils 0.18.1 which is incompatible.
ERROR:pep517.envbuild:great-expectations 0.14.13 requires jinja2<3.1.0,>=2.10, but you have jinja2 3.1.2 which is incompatible.
ERROR:pep517.envbuild:great-expectations 0.14.13 requires pyparsing<3,>=2.4, but you have pyparsing 3.0.9 which is incompatible.
ERROR:pep517.envbuild:flake8 4.0.1 requires importlib-metadata<4.3; python_version < "3.8", but you have importlib-metadata 4.11.4 which is incompatible.
ERROR:pep517.envbuild:feast 0.21.1.dev31+g46006d2e.d20220531 requires protobuf<3.20,>=3.10, but you have protobuf 3.20.1 which is incompatible.
ERROR:pep517.envbuild:WARNING: You are using pip version 22.0; however, version 22.1.2 is available.
ERROR:pep517.envbuild:You should consider upgrading via the '/Users/abhin/src/github.com/chhabrakadabra/feast/.venv37/bin/python -m pip install --upgrade pip' command.
Could not find a version that matches cryptography<37.0.0,==3.4.8,>=2.1.4,>=2.5,>=3.1.0,>=3.2,>=3.3.1,>=35.0 from https://files.pythonhosted.org/packages/00/e4/da1509e64a92e32ec8df97f5c4372e7f0e56b5b0bad299da61a9632b900c/cryptography-3.4.8-cp36-abi3-macosx_10_10_x86_64.whl#sha256=a00cf305f07b26c351d8d4e1af84ad7501eca8a342dedf24a7acb0e7b7406e14 (from feast (setup.py))
Tried: 0.1, 0.2, 0.2.1, 0.2.2, 0.3, 0.4, 0.5, 0.5.1, 0.5.2, 0.5.3, 0.5.4, 0.6, 0.6.1, 0.7, 0.7.1, 0.7.2, 0.8, 0.8.1, 0.8.2, 0.9, 0.9.1, 0.9.2, 0.9.3, 1.0, 1.0.1, 1.0.2, 1.1, 1.1.1, 1.1.2, 1.2, 1.2.1, 1.2.2, 1.2.3, 1.3, 1.3.1, 1.3.2, 1.3.3, 1.3.4, 1.4, 1.5, 1.5.1, 1.5.2, 1.5.3, 1.6, 1.7, 1.7.1, 1.7.2, 1.8, 1.8.1, 1.8.2, 1.9, 2.0, 2.0.1, 2.0.2, 2.0.3, 2.1, 2.1.1, 2.1.2, 2.1.3, 2.1.4, 2.2, 2.2, 2.2.1, 2.2.1, 2.2.2, 2.2.2, 2.3, 2.3, 2.3.1, 2.3.1, 2.4, 2.4, 2.4.1, 2.4.1, 2.4.2, 2.4.2, 2.5, 2.5, 2.6, 2.6.1, 2.6.1, 2.7, 2.7, 2.8, 2.8, 2.9, 2.9, 2.9.1, 2.9.1, 2.9.2, 2.9.2, 3.0, 3.0, 3.1, 3.1, 3.1.1, 3.1.1, 3.2, 3.2, 3.2.1, 3.2.1, 3.3, 3.3, 3.3.1, 3.3.1, 3.3.2, 3.3.2, 3.4, 3.4, 3.4, 3.4.1, 3.4.1, 3.4.2, 3.4.2, 3.4.3, 3.4.3, 3.4.4, 3.4.4, 3.4.5, 3.4.5, 3.4.6, 3.4.6, 3.4.7, 3.4.7, 3.4.8, 3.4.8, 35.0.0, 35.0.0, 36.0.0, 36.0.0, 36.0.0, 36.0.1, 36.0.1, 36.0.1, 36.0.2, 36.0.2, 36.0.2, 37.0.0, 37.0.0, 37.0.0, 37.0.1, 37.0.1, 37.0.1, 37.0.2, 37.0.2, 37.0.2
There are incompatible versions in the resolved dependencies:
  cryptography==3.4.8 from https://files.pythonhosted.org/packages/00/e4/da1509e64a92e32ec8df97f5c4372e7f0e56b5b0bad299da61a9632b900c/cryptography-3.4.8-cp36-abi3-macosx_10_10_x86_64.whl#sha256=a00cf305f07b26c351d8d4e1af84ad7501eca8a342dedf24a7acb0e7b7406e14 (from feast (setup.py))
  cryptography<37.0.0,>=3.1.0 (from snowflake-connector-python[pandas]==2.7.8->feast (setup.py))
  cryptography>=2.1.4 (from azure-storage-blob==12.12.0->adlfs==0.5.9->feast (setup.py))
  cryptography>=3.2 (from great-expectations==0.14.13->feast (setup.py))
  cryptography>=3.3.1 (from moto==3.1.11->feast (setup.py))
  cryptography>=2.5 (from azure-identity==1.10.0->adlfs==0.5.9->feast (setup.py))
  cryptography>=35.0 (from pyOpenSSL==22.0.0->snowflake-connector-python[pandas]==2.7.8->feast (setup.py))
make: *** [lock-python-ci-dependencies] Error 2

As far as I can tell, that error is legitimate. We legitimately pin cryptography to 3.4.8, which seems incompatible with the snowflake-connector-python[pandas] requirement of >=35.0. So what actually surprises me is not that this command fails, but why the other command even succeeds. I'm not 100% sure how this has ever worked in the past. I'd appreciate any help and guidance on this. Thanks.

@chhabrakadabra chhabrakadabra force-pushed the unpin-googleapis-common-protos branch from 577cc76 to f461992 Compare June 1, 2022 13:25
@feast-ci-bot feast-ci-bot added size/L and removed size/M labels Jun 1, 2022
chhabrakadabra and others added 3 commits June 1, 2022 18:09
@chhabrakadabra @adchia
Unpin googleapis-common-protos
7d4f512 
Exact pinning this library can cause Feast to be incompatible with other
libraries that users may want to install.

Signed-off-by: Abhin Chhabra <abhin.chhabra@shopify.com>
@chhabrakadabra @adchia
Regenerate requirements files.
13e6ce6 
Signed-off-by: Abhin Chhabra <chhabra.abhin@gmail.com>
@adchia
Fixing requirements
b664f0c 
Signed-off-by: Danny Chiao <danny@tecton.ai>
@adchia adchia force-pushed the unpin-googleapis-common-protos branch from 1ff6e95 to b664f0c Compare June 1, 2022 22:10
@adchia
Fix merge
4779176 
Signed-off-by: Danny Chiao <danny@tecton.ai>
adchia
adchia approved these changes Jun 1, 2022
View reviewed changes
Copy link
Collaborator

@adchia adchia left a comment

Choose a reason for hiding this comment

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

/lgtm

@feast-ci-bot
Copy link
Collaborator

feast-ci-bot commented Jun 1, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adchia, chhabrakadabra

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@feast-ci-bot feast-ci-bot merged commit e9784a1 into feast-dev:master Jun 2, 2022
@chhabrakadabra chhabrakadabra deleted the unpin-googleapis-common-protos branch June 2, 2022 16:17
@casassg
Copy link
Contributor

casassg commented Jun 10, 2022

Hey folks, any chance to do a patch release with this unpinning? Made super difficult to figure out why this failed: tensorflow/tfx-addons#152

@adchia
Copy link
Collaborator

adchia commented Jun 13, 2022

kicking off! sorry about that!

adchia added a commit that referenced this pull request Jun 13, 2022
@chhabrakadabra @adchia
chore: Unpin googleapis-common-protos (#2745)
d98a1ab 
* Unpin `googleapis-common-protos`

Exact pinning this library can cause Feast to be incompatible with other
libraries that users may want to install.

Signed-off-by: Abhin Chhabra <abhin.chhabra@shopify.com>

* Regenerate requirements files.

Signed-off-by: Abhin Chhabra <chhabra.abhin@gmail.com>

* Fixing requirements

Signed-off-by: Danny Chiao <danny@tecton.ai>

* Fix merge

Signed-off-by: Danny Chiao <danny@tecton.ai>

Co-authored-by: Danny Chiao <danny@tecton.ai>
@adchia
Copy link
Collaborator

adchia commented Jun 13, 2022

Release is out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adchia adchia adchia approved these changes

Assignees

@chhabrakadabra chhabrakadabra

@adchia adchia

Labels

approved lgtm ok-to-test size/L

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@chhabrakadabra @codecov-commenter @achals @feast-ci-bot @casassg @adchia @pyalex