Python release for 5.29.0 has indent problem

[thatch]

If you run python -m compileall on any of the wheels for the latest release ~40 minutes ago, it will fail with:

Compiling 'google/protobuf/service.py'...
*** Sorry: IndentationError: unindent does not match any outer indentation level (service.py, line 78)

I can confirm there appears to be a missing space, but I don't see that file (under that name or contents) checked in.

[piotlinski]

Confirming that, it seems to be visible on the 29.x branch:

protobuf/python/google/protobuf/service.py

Line 78 in e0a3978

raise NotImplementedError

[ejona86]

That file has been deleted on main, but it is in the 29.x branch.

[ElisaRMA]

Just FYI, still faced this issue today when importing some packages. The version installed was the 5.29.0. Reverting to the previous version solved the problem :)

[..]/google/protobuf/service.py:78 raise NotImplementedError ^ IndentationError: unindent does not match any outer indentation level

[SebKleiner]

Hi, I still face this issue while installing 5.29.0

[miguelgfierro]

Confirming as well, it's breaking all our tests: recommenders-team/recommenders#2191

Crazy idea: it would be nice to test your library before making a release.

[miguelgfierro]

@anandolee @ejona86 you need to yank 5.29.0 https://pypi.org/project/protobuf/5.29.0/

[dionhaefner]

lmao @ the fact that this is still live 5 days later

$ pip install protobuf
$ python -c "import google.protobuf.service"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/Users/dion/.virtualenvs/tempenv-2d0a294330027/lib/python3.10/site-packages/google/protobuf/service.py", line 78
    raise NotImplementedError
                             ^
IndentationError: unindent does not match any outer indentation level

[zhangskz]

This should be patched in v29.1 ~tomorrow. We are not planning on yanking v29.0 currently, since the issue is isolated to a long-deprecated API slated for removal. For now, we recommend not picking up v29.0 yet if you need compileall or if using the service.py (or ideally moving off of that since it'll be removed soon in v30.x).

Unfortunately the culprit change also removed the test that also would have caught it, as a side-effect of our removal of this API internally. We will consider adding tests with compileall to avoid similar test coverage problems in the future.

[miguelgfierro]

We are not planning on yanking v29.0 currently, since the issue is isolated to a long-deprecated API slated for removal.

@zhangskz It's not a matter whether the issue is deprecated or not, nobody can use that version in Python. That's the exact reason why it should be yanked. Is this serious?

[zhangskz]

This should only impact users importing the deprecated service.py (or usingcompileall) which should be relatively rare. We believe this shouldn't warrant a yank since most users should be able to use v29.0 successfully, where a yank may be disruptive as well.

I see in recommenders-team/recommenders#2191 that your project is indeed importing service.py. Apologies for the breakage, but would it suffice to simply pin to v28.x for now in the interim?

[nikhil-pandey]

Saw this issue with couple of projects using azure + mlflow

[miguelgfierro]

@zhangskz Recommenders doesn't have a dependency on protobuf, AzureML has. Basically, everybody doing machine learning with Microsoft products will have this error. I'm sure many other libraries will break, like mlflow.

I admire Google and Google engineers, but this is not the right way to operate. If you don't yank the release, you are hiding a major error, and everybody with a dependency in protobuf will get an error. In this end, this hurts your reputation as engineers and the reputation of Google.

Look, I've been coding for 20 years, and all of us have made mistakes. When I make a mistake, I fix it, I own the mistake and move on. I've been in your shoes many times, and I understand it's not nice to yank a release. But doing it fortifies your library.

I hope you do the right thing.

[jkoessle]

Saw this issue with couple of projects using azure + mlflow

Same for us. Its not possible to use 5.29.0 and mlflow in our ML pipelines.

[KrystianOcado]

This should only impact users importing the deprecated service.py (or usingcompileall) which should be relatively rare. We believe this shouldn't warrant a yank since most users should be able to use v29.0 successfully, where a yank may be disruptive as well.

I see in recommenders-team/recommenders#2191 that your project is indeed importing service.py. Apologies for the breakage, but would it suffice to simply pin to v28.x for now in the interim?

Hi @zhangskz !

You don't need to import "service.py" to experience this error. GCP Cloud Function compiles all the code when you deploy new function, so even if you don't use "service.py" you see this error. Please yank the broken version.

Thanks!

[luisangelico]

Having this issue with Mlflow on Azure.
pip install protobuf==5.28.0 fixed it

[googleberg]

Hello all,

I know that issues like this can be frustrating. No one likes to have their work blocked by a breakage. Sorry about that.

It is also important to recognize that not every user's use case is the same as yours. As @zhangskz points out, while a yank might seem like an obvious solution, there are other users that may have already latched onto other working parts of 29.0 and yanking may break them.

When formulating your replies, keep in mind that protobuf has a Code of Conduct. We do want feedback and suggestions but we also expect civility and mutual respect.

We will be releasing a patch that resolves this issue shortly.
We will be adding additional release testing to ensure that even deprecated functionality like service.py is not broken in future releases.

Thank you for your patience.

[imatiach-msft]

+1 seeing this as well, this is breaking everything for me - great to see some familiar faces here 👋

[eltoder]

@zhangskz running compileall is not at all rare. It is a standard step when deploying your code.

[dionhaefner]

[d8ahazard]

Hello all,

I know that issues like this can be frustrating. No one likes to have their work blocked by a breakage. Sorry about that.

It is also important to recognize that not every user's use case is the same as yours. As @zhangskz points out, while a yank might seem like an obvious solution, there are other users that may have already latched onto other working parts of 29.0 and yanking may break them.

When formulating your replies, keep in mind that protobuf has a Code of Conduct. We do want feedback and suggestions but we also expect civility and mutual respect.

We will be releasing a patch that resolves this issue shortly. We will be adding additional release testing to ensure that even deprecated functionality like service.py is not broken in future releases.

Thank you for your patience.

Respectfully, you guys need to move NOW.

Why is the broken package still published, and why have you not release a fix?

Considering how many other packages use protobuf, this should have been addressed last week when it was reported...not "maybe some time today.".

[pberenyi]

Maybe you want to add something like python -m compileall -q . into your release verification process?

[karpikpl]

Looks like 5.29.1 was released

[jorenham]

Looks like 5.29.1 was released

It's not yet on PyPI:
https://pypi.org/project/protobuf/

[jorenham]

I would suggest yanking the 5.29.0 PyPI release.

[jorenham]

Respectfully, you guys need to move NOW.

I strongly agree. This causes major disruptions in a many CI/CD pipelines.
Do not underestimate the damage you are causing by ignoring this.

[ericsalo]

We are not ignoring it. We are proceeding to release 29.1 as quickly as we can, and once that is done we will yank 29.0.

…

On Wed, Dec 4, 2024 at 1:28 PM Joren Hammudoglu ***@***.***> wrote: Respectfully, you guys need to move NOW. I strongly agree. This causes major disruptions in a many CI/CD pipelines. Do not underestimate the damage you are causing by ignoring this. — Reply to this email directly, view it on GitHub <#19430 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AWHIXARA2WEZ4IZNB4CANWD2D5JVZAVCNFSM6AAAAABSTVNCZWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKMJYGM4DAMJTGY> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[jorenham]

Protobuf 5.29.1 is on PyPI now:
https://pypi.org/project/protobuf/

[jorenham]

And Protobbuf 5.29.0 has indeed been yanked:
https://pypi.org/project/protobuf/5.29.0/

Thanks @ericsalo

[miguelgfierro]

Thanks for yanking @ericsalo @zhangskz @googleberg
Respect 👏👏👏

[thatch]

Closing as fixed version is out. Thanks for addressing, protobuf team.