Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Pin protobuf<=3.20.1 #13177

Merged
merged 4 commits into from
May 31, 2022
Merged

Pin protobuf<=3.20.1 #13177

merged 4 commits into from
May 31, 2022

Conversation

akihironitta
Copy link
Contributor

@akihironitta akihironitta commented May 28, 2022 •
edited by carmocca
Loading

What does this PR do?

Fixes #13159.
Closes #13189

Does your PR introduce any breaking changes? If yes, please list them.

None

Before submitting

  • Was this discussed/approved via a GitHub issue? (not for typos and docs)
  • Did you read the contributor guideline, Pull Request section?
  • [n/a] Did you make sure your PR does only one thing, instead of bundling different changes together?
  • [n/a] Did you make sure to update the documentation with your changes? (if necessary)
  • [n/a] Did you write any new necessary tests? (not for typos and docs)
  • [n/a] Did you verify new and existing tests pass locally with your changes?
  • Did you list all the breaking changes introduced by this pull request?
  • Did you update the CHANGELOG? (not for typos, docs, test updates, or minor internal changes/refactors)

PR review

Anyone in the community is welcome to review the PR.
Before you start reviewing, make sure you have read the review guidelines. In short, see the following bullet-list:

  • Is this pull request ready for review? (if not, please submit in draft mode)
  • Check that all items from Before submitting are resolved
  • Make sure the title is self-explanatory and the description concisely explains the PR
  • Add labels and milestones (and optionally projects) to the PR so it can be classified

Did you have fun?

Make sure you had fun coding 🙃

cc @tchaton @rohitgr7 @carmocca @akihironitta @Borda

@akihironitta akihironitta added priority: 0 High priority task ci Continuous Integration labels May 28, 2022
@akihironitta akihironitta added this to the 1.6.x milestone May 28, 2022
@akihironitta akihironitta force-pushed the req/pin-protobuf branch 2 times, most recently from f62471d to 4e541f1 Compare May 28, 2022 21:54
@akihironitta akihironitta changed the title Pin protobuf<4.21.0 Pin protobuf<=3.20.1 May 28, 2022
@akihironitta
Copy link
Contributor Author

@Borda I have no idea why the installation test installs the latest version even though I explicitly specified protobuf<=3.20.1 in the base requirements file. Do you have any idea?

...
Collecting protobuf
  Downloading protobuf-4.21.1-cp37-abi3-manylinux2014_x86_64.whl (407 kB)
...

https://github.com/PyTorchLightning/pytorch-lightning/runs/6639424023?check_suite_focus=true

@mergify mergify bot added the ready PRs ready to be merged label May 29, 2022
@Borda
Copy link
Member

Borda commented May 29, 2022

Do you have any idea?

it is coming from a dependency chain... and we install all with -U

carmocca
carmocca approved these changes May 30, 2022
View reviewed changes
requirements/base.txt Outdated Show resolved Hide resolved
@akihironitta
Copy link
Contributor Author

I have no idea why the installation test installs the latest version even though I explicitly specified protobuf<=3.20.1 in the base requirements file.

It turns out that the issue is with pytorch_lightning.setup_tools._load_requirements. Will debug it and include the patch here.

@carmocca
Copy link
Contributor

Let's also skip the quantization tests here to unblock.

@carmocca carmocca force-pushed the req/pin-protobuf branch 2 times, most recently from 31a4b6d to b62add5 Compare May 30, 2022 18:25
@carmocca carmocca enabled auto-merge (rebase) May 30, 2022 18:25
@carmocca carmocca force-pushed the req/pin-protobuf branch 2 times, most recently from 3261217 to 0e1e493 Compare May 30, 2022 21:18
@Borda
Copy link
Member

Borda commented May 31, 2022

how about this fix: #13189

@carmocca carmocca mentioned this pull request May 31, 2022
Closed
@carmocca carmocca merged commit 3780407 into master May 31, 2022
@carmocca carmocca deleted the req/pin-protobuf branch May 31, 2022 18:21
carmocca pushed a commit that referenced this pull request May 31, 2022
@akihironitta @carmocca
Pin protobuf version (#13177)
40dddc8
carmocca added a commit that referenced this pull request May 31, 2022
@carmocca
xfail flaky quantization test blocking CI (#13177)
50326e9
carmocca added a commit that referenced this pull request May 31, 2022
@carmocca
Fix standalone test collection (#13177)
9945d02
carmocca pushed a commit that referenced this pull request May 31, 2022
@akihironitta @carmocca
Pin protobuf version (#13177)
e66f8ad
carmocca added a commit that referenced this pull request May 31, 2022
@carmocca
xfail flaky quantization test blocking CI (#13177)
46b9dbe
carmocca added a commit that referenced this pull request May 31, 2022
@carmocca
Fix standalone test collection (#13177)
9c6ee04
carmocca added a commit that referenced this pull request May 31, 2022
@carmocca
Revert "Update deepspeed requirement from <0.6.0 to <0.7.0 in /requir…
378c4de 
…ements (#13048)" (#13177)

This reverts commit e19babc.
carmocca pushed a commit that referenced this pull request May 31, 2022
@akihironitta @carmocca
Pin protobuf version (#13177)
7080ab2
carmocca added a commit that referenced this pull request May 31, 2022
@carmocca
xfail flaky quantization test blocking CI (#13177)
6226525
carmocca added a commit that referenced this pull request May 31, 2022
@carmocca
Fix standalone test collection (#13177)
92f3b30
carmocca added a commit that referenced this pull request May 31, 2022
@carmocca
Revert "Update deepspeed requirement from <0.6.0 to <0.7.0 in /requir…
c9d49fe 
…ements (#13048)" (#13177)

This reverts commit e19babc.
lexierule pushed a commit that referenced this pull request Jun 1, 2022
@akihironitta @lexierule
Pin protobuf version (#13177)
91003c9
lexierule pushed a commit that referenced this pull request Jun 1, 2022
@carmocca @lexierule
xfail flaky quantization test blocking CI (#13177)
e21219d
lexierule pushed a commit that referenced this pull request Jun 1, 2022
@carmocca @lexierule
Fix standalone test collection (#13177)
7e52126
lexierule pushed a commit that referenced this pull request Jun 1, 2022
@carmocca @lexierule
Revert "Update deepspeed requirement from <0.6.0 to <0.7.0 in /requir…
3c06cd8 
…ements (#13048)" (#13177)

This reverts commit e19babc.
@ricardorei ricardorei mentioned this pull request Jun 1, 2022
Closed
@carmocca carmocca mentioned this pull request Jun 29, 2022
Closed
8 tasks
@rohitgr7 rohitgr7 mentioned this pull request Jul 1, 2022
Merged
12 tasks
This was referenced Sep 11, 2022
Open
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@awaelchli awaelchli awaelchli approved these changes

@Borda Borda Borda approved these changes

@carmocca carmocca carmocca approved these changes

@williamFalcon williamFalcon Awaiting requested review from williamFalcon

@tchaton tchaton Awaiting requested review from tchaton tchaton is a code owner

@SeanNaren SeanNaren Awaiting requested review from SeanNaren

@justusschock justusschock Awaiting requested review from justusschock justusschock is a code owner

@kaushikb11 kaushikb11 Awaiting requested review from kaushikb11

@rohitgr7 rohitgr7 Awaiting requested review from rohitgr7

Assignees
No one assigned
Labels
ci Continuous Integration priority: 0 High priority task ready PRs ready to be merged
Projects
None yet
Milestone
pl:1.6.x
Development

Successfully merging this pull request may close these issues.

import error with protobuf==4.21.0
4 participants
@akihironitta @Borda @carmocca @awaelchli