How should protobuf Python be packaged to coexist with other packages in the google namespace? #1296
Comments
|
Currently I've hacked this to work locally by going into the This is very far from optimal and I hope we can work towards a solution! |
Improves protocolbuffers#1296. The problem: in the previous patch, we tweaked the __init__.py files to use namespaces, but no longer declared ourselves as a namespace package. The second half was unwise. Note that this only comes up when installing protobuf alongside another package that also installs into the google namespace; as of right now, the only PyPI package that does is googleapis-common-protos, though the GAE SDK also uses google.appengine. Installing either or both of those alongside this package now works. The case that still remains is the upgrade path, which is also what worried me in protocolbuffers#713. It seems that if protobuf 2.6.1 is installed, there's no way to safely upgrade that to work with a newer protobuf. However, `pip uninstall` && `pip install` does the trick.
|
Every python package that wants to use the Add # this is a namespace package
try:
import pkg_resources
pkg_resources.declare_namespace(__name__)
except ImportError:
import pkgutil
__path__ = pkgutil.extend_path(__path__, __name__)( Add setup(
name='protobuf',
namespace_packages=['google'],
...
)If any one package doesn't do this and is installed alongside the others, it's very likely to completely break the installation. We should get a list of all known packages and ensure they conform to this. |
|
With the other PR merged, we're now at least back to a reasonable working state. (Famous last words?) I think the pending things we should do before closing out this PR include:
Also, @jonparrott -- what was the rest of the the sentence you started with " |
|
I got some conflicting information here. ¯_(ツ)_/¯
I think what I've recommended above is fine, and it is what's used in other common namespace packages (namely zope). @tseaver might be able to shed some insight there as well. |
|
FWIW, packages in the __import__('pkg_resources').declare_namespace(__name__) |
|
@craigcitro for the in-place upgrade, you can actually check in setup.py if an old version is installed and issue a warning saying to remove then install instead of upgrade. I've seen another package do this. |
|
@jonparrott sweet -- if there's prior art, I think that's the right fix for the upgrade path. @haberman does that sound reasonable to you? |
|
so whats the solution now? if i install protobuf over pip no init file is placed inside the google folder, how can i import protobuf? |
|
The presence of a |
|
@craigcitro If there is a possible solution to this problem by all means let's move forward with it! |
|
AFAIK, the current version of protobuf installs cleanly in the $ $VENV/bin/pip freeze
-f file:///home/tseaver/.pip/wheels
enum34==1.1.6
future==0.15.2
futures==3.0.5
gax-google-logging-v2==0.8.1
gax-google-pubsub-v1==0.8.1
gcloud==0.18.0
google-gax==0.12.5
googleapis-common-protos==1.2.0
grpc-google-logging-v2==0.8.1
grpc-google-pubsub-v1==0.8.1
grpcio==1.0.0rc2
httplib2==0.9.2
nose==1.3.7
nose-exclude==0.4.1
oauth2client==3.0.0
ply==3.8
protobuf==3.0.0
pyasn1==0.1.9
pyasn1-modules==0.0.8
rsa==3.4.2
six==1.10.0
$ for sub in $(ls $VENV/lib/python2.7/site-packages/google); do
echo $sub
$VENV/bin/python -c "import google.$sub"
done
api
cloud
gax
logging
longrunning
protobuf
pubsub
rpc
type |
|
Should this issue still be open? It seems like the usage of |
|
@glyph @haberman It's maybe reasonable to close this issue; however, the work around the upgrade path from this comment still remains. Do we want to just move that to a new issue? In particular, we need to:
|
|
@craigcitro - Bailing in |
No, it's better than bad, it's good :). Trust me - as somebody who has spent roughly a decade of my life debugging weird In other words: the small detriment to you is that you can't run code in setup.py, but the tremendous benefit to you is that nobody else can run code in setup.py either :).
It'd probably be better and more helpful to contribute upstream to something like |
Oh, sorry, I was unclear there -- I meant it was bad in the context of trying to detect this issue at install time. I totally agree that it's great for everyone's sanity; I have many fewer
Definitely -- but unfortunately I don't have time for that at the moment. For now, we're going to just wait and see if this causes us friction, but if it does, that's probably the best way to invest our time wrt fixing it.
Well, in this case, |
|
Yep, very reasonable work around.
Importing protobuf first works because protobuf will setup the namespace
package. Importing appengine first makes python stop looking elsewhere for
google namespace packages.
…On Thu, Dec 1, 2016 at 11:13 AM Evan Jones ***@***.***> wrote:
I hate to be that random Internet guy commenting unhelpfully on a bug, but
we ran into this while trying to use this library with the App Engine SDK.
With the current release (3.1.0.post1), we had fun issues where import
google.appengine would fail, but import google.protobuf; import
google.appengine would work, presumably due to some namespace package
magic on Python 2.7. We fixed it by adding a gae.pth file to our local
site-packages that forces google to be a namespace package: import
google; import pkgutil; pkgutil.extend_path(google.__path__,
google.__name__). Until App Engine updates its code, this seems like a
reasonable workaround.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1296 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAPUc9htGxRs_3RVdW33hL9cVQ6_GRYBks5rDxxZgaJpZM4Hp170>
.
|
|
@jonparrott is there already an issue filed against the GAE side to fix that? (Or should we file one?) |
|
I'm going to close this issue, please re-open if this requires any specific action from our side. |
Related to protocolbuffers#1296: The issue seems to be fixed for consumers of the python protobuf package from pypi, but not for anyone getting it from here as a Bazel remote repository.
Protobuf needs to be installed from source to workaround protocolbuffers/protobuf#1296
This could happen with any package in the `google` namespace, but protobuf is the only one I've ever seen in the wild See protocolbuffers/protobuf#1296 for details
|
Hi As far as I am concerned, google protobuf is installed in conjunction with the tensorflow transform as a dependency. I (tried to) follow the workarounds suggested, but couldn't manage to get the project working. Is it possible for someone to write a clean workaround for this so that it can be followed easily? |
|
@ShahinNamin new versions of all the libraries involved should be well behaved. What does |
|
it has installed protobuf 1.3.4 google cloud storage 1.4.0 and tensorflow 1.4.0. I removed the tensorflow and protobuf and it seems the problem persists, so it seems it not the protobuf issue. |
|
@ShahinNamin I would recommend trying to come up with a minimal repro against current versions of libraries, and reporting a new issue with the library or libraries that cause conflict; this particular issue is long fixed. 😁 (Protobuf is at 3.4.0.) |
|
I can't believe I found this: |
|
I get the same issue (#1296 (comment)) when using protobuf 3.5.1. There's no " |
|
@haberman I'd like to re-open this, but seems I don't have permission to do that. Could you help? |
|
Please don't reopen a two-year-old bug. Just open a new one and reference this. |
This might really be a bug in protobuf-python, protocolbuffers/protobuf#1296
In protocolbuffers#713 and protocolbuffers#1296, the `google` package in protobuf sources was found to cause conflicts with other Google projects, because it was not properly configured as a namespace package [1]. The initial fix in 786f80f addressed part of the issue, and protocolbuffers#1298 fixed the rest. However, 786f80f (the initial fix) also made `google.protobuf` and `google.protobuf.pyext` into namespace packages. This was not correct: they are both regular, non-namespace, sub-subpackages. However (still), the follow-up protocolbuffers#1298 did not nominate them as namespace packages, so the namespace registration behavior has remained, but without benefit. This change removes the unnecessary namespace registration, which has substantial overhead, thus reducing startup time substantially when using protobufs. Because this change affects the import internals, quantifying the overhead requires a full tear-down/start-up of the Python interpreter. So, to capture the full cost for every run, I measured the time to launching a _fresh_ Python instance in a subprocess, varying the imports and code under test. In other words, I used `timeit` to measure the time to launch a _fresh_ Python subprocess which actually performs the imports. * Reference: normal Python startup (i.e., don't import protobuf at all). ``` % python3 -m timeit -s 'import subprocess' -r 3 -n 10 'subprocess.call(["python3", "-c", "pass"])' 10 loops, best of 3: 27.1 msec per loop ``` * Baseline: cost to import `google.protobuf.descriptor`, with extraneous namespace packages. ``` % python3 -m timeit -s 'import subprocess' -r 3 -n 10 'subprocess.call(["python3", "-c", "import google.protobuf.descriptor"])' 10 loops, best of 3: 133 msec per loop ``` * This change: cost to import `google.protobuf.descriptor`, without extraneous namespace packages. ``` % python3 -m timeit -s 'import subprocess' -r 3 -n 10 'subprocess.call(["python3", "-c", "import google.protobuf.descriptor"])' 10 loops, best of 3: 43.1 msec per loop ``` [1]: https://packaging.python.org/guides/packaging-namespace-packages/
In #713 and #1296, the `google` package in protobuf sources was found to cause conflicts with other Google projects, because it was not properly configured as a namespace package [1]. The initial fix in 786f80f addressed part of the issue, and #1298 fixed the rest. However, 786f80f (the initial fix) also made `google.protobuf` and `google.protobuf.pyext` into namespace packages. This was not correct: they are both regular, non-namespace, sub-subpackages. However (still), the follow-up #1298 did not nominate them as namespace packages, so the namespace registration behavior has remained, but without benefit. This change removes the unnecessary namespace registration, which has substantial overhead, thus reducing startup time substantially when using protobufs. Because this change affects the import internals, quantifying the overhead requires a full tear-down/start-up of the Python interpreter. So, to capture the full cost for every run, I measured the time to launching a _fresh_ Python instance in a subprocess, varying the imports and code under test. In other words, I used `timeit` to measure the time to launch a _fresh_ Python subprocess which actually performs the imports. * Reference: normal Python startup (i.e., don't import protobuf at all). ``` % python3 -m timeit -s 'import subprocess' -r 3 -n 10 'subprocess.call(["python3", "-c", "pass"])' 10 loops, best of 3: 27.1 msec per loop ``` * Baseline: cost to import `google.protobuf.descriptor`, with extraneous namespace packages. ``` % python3 -m timeit -s 'import subprocess' -r 3 -n 10 'subprocess.call(["python3", "-c", "import google.protobuf.descriptor"])' 10 loops, best of 3: 133 msec per loop ``` * This change: cost to import `google.protobuf.descriptor`, without extraneous namespace packages. ``` % python3 -m timeit -s 'import subprocess' -r 3 -n 10 'subprocess.call(["python3", "-c", "import google.protobuf.descriptor"])' 10 loops, best of 3: 43.1 msec per loop ``` [1]: https://packaging.python.org/guides/packaging-namespace-packages/
I'm creating this as a master issue to track this long-standing problem. Several people have encountered this problem, but no one seems to know the correct way to solve it.
Here are several issues that say that installing the protobuf package makes other Python packages under the
google.namespace inaccessible because protobuf has no__init__.py. All of these issues were reported against version<= 3.0.0b2: #713 #859 #1153 #1272These issues were supposed to be fixed by this PR: #1259 That PR was integrated into a new release on PyPI, 3.0.0b2.post1 (https://pypi.python.org/pypi/protobuf/3.0.0b2.post1).
However that PR caused a different bug: #1294 googleapis/google-cloud-python#1570
We need a solution to this problem, but finding a solution that works has proven to be elusive. Would very much appreciate anyone who can propose a fix they are confident will fix all of these problems.
The text was updated successfully, but these errors were encountered: