Fix #215: Helper for int and double now work as expected.

[jgeewax]

No description provided.

[jgeewax]

@tseaver

[tseaver]

As I noted on #215, I don't believe this change is necessary. Current releases of protobuf do return the (potentially coerced) value from the checker.

[jgeewax]

Ah you're right. Didn't realize that. However, after some digging...

I'm on Ubuntu 13.10, with the python-protobuf package installed (via apt, not pip) and it returns None :( This makes me think the patch might be a good idea...

(master)jj@jjg-gigabyte:~/projects/gcloud$ sudo apt-get install python-protobuf
Reading package lists... Done
Building dependency tree       
Reading state information... Done
python-protobuf is already the newest version.
The following packages were automatically installed and are no longer required:
  libgetopt-argvfile-perl libmodule-scandeps-perl libmodule-signature-perl libpar-dist-perl libpar-perl python-keyring python-launchpadlib python-lazr.restfulclient python-lazr.uri python-oauth python-problem-report python-secretstorage python-simplejson python-wadllib
Use 'apt-get autoremove' to remove them.
0 upgraded, 0 newly installed, 0 to remove and 6 not upgraded.
(master)jj@jjg-gigabyte:~/projects/gcloud$ dpkg -s python-protobuf
Package: python-protobuf
Status: install ok installed
Priority: optional
Section: python
Installed-Size: 458
Maintainer: Ubuntu Developers <[email protected]>
Architecture: amd64
Source: protobuf
Version: 2.4.1-3ubuntu2
Provides: python2.7-protobuf
Depends: libc6 (>= 2.4), libgcc1 (>= 1:4.1.1), libprotobuf7, libstdc++6 (>= 4.1.1), python (>= 2.7), python (<< 2.8), python:any (>= 2.7.1-0ubuntu2)
Recommends: protobuf-compiler
Description: Python bindings for protocol buffers
 Protocol buffers are a flexible, efficient, automated mechanism for
 serializing structured data - similar to XML, but smaller, faster, and
 simpler. You define how you want your data to be structured once, then you can
 use special generated source code to easily write and read your structured
 data to and from a variety of data streams and using a variety of languages.
 You can even update your data structure without breaking deployed programs
 that are compiled against the "old" format.
 .
 Google uses Protocol Buffers for almost all of its internal RPC protocols and
 file formats.
 .
 This package contains the Python bindings for the protocol buffers. You will
 need the protoc tool (in the protobuf-compiler package) to compile your
 definition to Python classes, and then the modules in this package will allow
 you to use those classes in your programs.
 .
 This package contains both the traditional Python-based
 implementation and the new C++-based one, and you can select at
 runtime between the two.
Homepage: http://code.google.com/p/protobuf/
Original-Maintainer: Iustin Pop <[email protected]>
(master)jj@jjg-gigabyte:~/projects/gcloud$ python 
Python 2.7.5+ (default, Feb 27 2014, 19:37:08) 
[GCC 4.8.1] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> from google import protobuf
>>> from google.protobuf.internal.type_checkers import Int64ValueChecker
>>> print Int64ValueChecker().CheckValue(123)
None

[dhermes]

Ubuntu 13.10 is not an LTS release (12.04 and 14.04 are nearest). That is possibly the reason for out of date packages.

UPDATE: 13.10 was EOL'd in July - https://wiki.ubuntu.com/Releases

UPDATE (Again): The versions on the LTS releases are:

• The 14.04 packages refer to version 2.5.0
• The 12.04 packages refer to version 2.4.1
• The 10.04 packages refer to version 2.2.0 (EOL'd for Desktop in 2013, April 2015 is server EOL)

But at the end of the day, the version in PyPI is up to date, so we need not really worry (other than the version conflict with dateutil I previously mentioned).

[tseaver]

Centos / RHEL 6: python-protobuf-2.3.0
Centos / RHEL 7 / all current Fedoras: python-protobuf-2.5.0

As I noted on #215, looks like we either need to pin our dependency ('protobuf >= 2.6.0') or else apply the patch to handle older protobuf versions.

[aliafshar]

I'm inclined to go with pinning the dependency. Using the system Python site library is not the way to run an app on any platform. The question we should ask is: does 2.6 run on RHEL6 etc?

With one caveat, we make it very clear what the problem is when an earlier library version is installed. Returning None unexpectedly is the most malicious outcome a piece of Python code can do. Suggest: check library version somewhere in the code and explicitly give an error.

[jgeewax]

I'm +1 on pinning the dependency, and +1 on being compatible with the old CheckValue method...

I have a hunch this will be a really subtle error, and since Google did some weird stuff with the protobuf package namespacing (google.protobuf), it's just extra headache IMO.

For reference, here's what happens when I try to install the newer version of protobuf (and therefore what will happen if I do pip install -r requirements.txt based on a pinned version):

(master)jj@jjg-gigabyte:~/projects/gcloud$ sudo pip install protobuf --upgrade
Downloading/unpacking protobuf from https://pypi.python.org/packages/source/p/protobuf/protobuf-2.6.0.tar.gz#md5=3a25a1efda4feba5427b96e9b34bc94a
  Downloading protobuf-2.6.0.tar.gz (187kB): 187kB downloaded
  Running setup.py egg_info for package protobuf
    zip_safe flag not set; analyzing archive contents...
    google.__init__: module references __path__
    google.apputils.setup_command: module references __file__
    google.apputils.run_script_module: module references __file__
    google.apputils.__init__: module references __path__

    Installed /tmp/pip_build_root/protobuf/google_apputils-0.4.0-py2.7.egg
    Searching for python-gflags>=1.4
    Reading http://pypi.python.org/simple/python-gflags/
    Reading http://code.google.com/p/python-gflags
    Best match: python-gflags 2.0
    Downloading https://pypi.python.org/packages/source/p/python-gflags/python-gflags-2.0.tar.gz#md5=23c9a793959a54971b1f094b0c6d03b1
    Processing python-gflags-2.0.tar.gz
    Writing /tmp/easy_install-sBhHBF/python-gflags-2.0/setup.cfg
    Running python-gflags-2.0/setup.py -q bdist_egg --dist-dir /tmp/easy_install-sBhHBF/python-gflags-2.0/egg-dist-tmp-2iR1Tn
    zip_safe flag not set; analyzing archive contents...

    Installed /tmp/pip_build_root/protobuf/python_gflags-2.0-py2.7.egg
    Traceback (most recent call last):
      File "<string>", line 16, in <module>
      File "/tmp/pip_build_root/protobuf/setup.py", line 199, in <module>
        "Protocol Buffers are Google's data interchange format.",
      File "/usr/lib/python2.7/distutils/core.py", line 112, in setup
        _setup_distribution = dist = klass(attrs)
      File "/usr/lib/python2.7/dist-packages/setuptools/dist.py", line 221, in __init__
        self.fetch_build_eggs(attrs.pop('setup_requires'))
      File "/usr/lib/python2.7/dist-packages/setuptools/dist.py", line 245, in fetch_build_eggs
        parse_requirements(requires), installer=self.fetch_build_egg
      File "/usr/lib/python2.7/dist-packages/pkg_resources.py", line 598, in resolve
        raise VersionConflict(dist,req) # XXX put more info here
    pkg_resources.VersionConflict: (python-dateutil 2.2 (/usr/local/lib/python2.7/dist-packages), Requirement.parse('python-dateutil>=1.4,<2'))
    Complete output from command python setup.py egg_info:
    zip_safe flag not set; analyzing archive contents...

google.__init__: module references __path__

google.apputils.setup_command: module references __file__

google.apputils.run_script_module: module references __file__

google.apputils.__init__: module references __path__



Installed /tmp/pip_build_root/protobuf/google_apputils-0.4.0-py2.7.egg

Searching for python-gflags>=1.4

Reading http://pypi.python.org/simple/python-gflags/

Reading http://code.google.com/p/python-gflags

Best match: python-gflags 2.0

Downloading https://pypi.python.org/packages/source/p/python-gflags/python-gflags-2.0.tar.gz#md5=23c9a793959a54971b1f094b0c6d03b1

Processing python-gflags-2.0.tar.gz

Writing /tmp/easy_install-sBhHBF/python-gflags-2.0/setup.cfg

Running python-gflags-2.0/setup.py -q bdist_egg --dist-dir /tmp/easy_install-sBhHBF/python-gflags-2.0/egg-dist-tmp-2iR1Tn

zip_safe flag not set; analyzing archive contents...



Installed /tmp/pip_build_root/protobuf/python_gflags-2.0-py2.7.egg

Traceback (most recent call last):

  File "<string>", line 16, in <module>

  File "/tmp/pip_build_root/protobuf/setup.py", line 199, in <module>

    "Protocol Buffers are Google's data interchange format.",

  File "/usr/lib/python2.7/distutils/core.py", line 112, in setup

    _setup_distribution = dist = klass(attrs)

  File "/usr/lib/python2.7/dist-packages/setuptools/dist.py", line 221, in __init__

    self.fetch_build_eggs(attrs.pop('setup_requires'))

  File "/usr/lib/python2.7/dist-packages/setuptools/dist.py", line 245, in fetch_build_eggs

    parse_requirements(requires), installer=self.fetch_build_egg

  File "/usr/lib/python2.7/dist-packages/pkg_resources.py", line 598, in resolve

    raise VersionConflict(dist,req) # XXX put more info here

pkg_resources.VersionConflict: (python-dateutil 2.2 (/usr/local/lib/python2.7/dist-packages), Requirement.parse('python-dateutil>=1.4,<2'))

----------------------------------------
Cleaning up...
Command python setup.py egg_info failed with error code 1 in /tmp/pip_build_root/protobuf
Storing complete log in /home/jj/.pip/pip.log

I had to do....

$ sudo pip uninstall python-dateutil
$ sudo pip install "python-dateutil < 2"
$ sudo pip install protobuf

to get things to work.. this is far from a good experience just to get the right version of protobuf on a machine :(

[dhermes]

@jgeewax This dependency is a problem with protobuf and IMO should be raised with the authors.

I brought this up in #91 and also pointed out a thread on their forum.

Also, if we are pinning the dependency, then why worry about backwards compatibility?

[aliafshar]

@dhermes we worry, because setup.py is not the only way to install things.

[dhermes]

This is my current feeling(s) on the matter.

@aliafshar How does one control the other ways to install things in a new release? (i.e. some of the broken things are already out there and not easy to pull back in / revise.)

[jgeewax]

I've updated setup.py to pin protobuf at 2.5.0 per our discussion on #215. I think this is the best "solution" to the problem given where we are today.

[coveralls]

Coverage increased (+0.0%) when pulling 02402f4 on jgeewax:master into 1779aec on GoogleCloudPlatform:master.

[tseaver]

@aliafshar what other ways do we care about? .deb and .rpm packagers will use the 'install_requires' from setup.py to compute dependencies on the relevant packaged libraries. If we are worrying about developers who have older versions of protobuf installed in their system site-pacakges (e.g., via .deb or .rpm), then ISTM we need to teach them about working in a virtualenv, isolating any cruft in /usr/lib.

[tseaver]

@jgeewax Pinning protobuf at 2.5.0 can't be the right thing: even if we merge your patch, the code still works fine with 2.6.0.

[jgeewax]

So how about protobuf >= 2.5.0 ? That way, older Ubuntu versions don't require a newer version (which gives lots of errors while installing) and newer versions are happy. Sound good?

[dhermes]

protobuf==2.6.0 (i.e. latest) will not install in its current state, which makes this library uninstallable. For devs who have no idea how to hack around the protobuf install, this is a problem.

[tseaver]

@dhermes I can't replicate that in a clean virtualenv (tox wouldn't be passing, either):

$ /opt/Python-2.7.6/bin/virtualenv /tmp/protobuf
New python executable in /tmp/protobuf/bin/python
Installing setuptools, pip...done.
$ /tmp/protobuf/bin/pip install protobuf
Downloading/unpacking protobuf
Downloading protobuf-2.6.0.tar.gz (187kB): 187kB downloaded
Running setup.py (path:/tmp/protobuf/build/protobuf/setup.py) egg_info for package protobuf
    zip_safe flag not set; analyzing archive contents...
    google.__init__: module references __path__
    google.apputils.__init__: module references __path__
    google.apputils.run_script_module: module references __file__
    google.apputils.setup_command: module references __file__

    Installed /tmp/protobuf/build/protobuf/google_apputils-0.4.0-py2.7.egg
    Searching for pytz>=2010
    Reading https://pypi.python.org/simple/pytz/
    Best match: pytz 2014.7
    Downloading https://pypi.python.org/packages/2.7/p/pytz/pytz-2014.7-py2.7.egg#md5=316ea05e4fd43ae88b2b559cacacf81b
    Processing pytz-2014.7-py2.7.egg
    Moving pytz-2014.7-py2.7.egg to /tmp/protobuf/build/protobuf

    Installed /tmp/protobuf/build/protobuf/pytz-2014.7-py2.7.egg
    Searching for python-gflags>=1.4
    Reading https://pypi.python.org/simple/python-gflags/
    Reading http://code.google.com/p/python-gflags
    Best match: python-gflags 2.0
    Downloading https://pypi.python.org/packages/source/p/python-gflags/python-gflags-2.0.tar.gz#md5=23c9a793959a54971b1f094b0c6d03b1
    Processing python-gflags-2.0.tar.gz
    Writing /tmp/easy_install-MbrsQ0/python-gflags-2.0/setup.cfg
    Running python-gflags-2.0/setup.py -q bdist_egg --dist-dir /tmp/easy_install-MbrsQ0/python-gflags-2.0/egg-dist-tmp-mY68Te
    zip_safe flag not set; analyzing archive contents...

    Installed /tmp/protobuf/build/protobuf/python_gflags-2.0-py2.7.egg
    Searching for python-dateutil>=1.4,<2
    Reading https://pypi.python.org/simple/python-dateutil/
    Reading http://labix.org/python-dateutil
    Best match: python-dateutil 1.5
    Downloading https://pypi.python.org/packages/source/p/python-dateutil/python-dateutil-1.5.tar.gz#md5=0dcb1de5e5cad69490a3b6ab63f0cfa5
    Processing python-dateutil-1.5.tar.gz
    Writing /tmp/easy_install-Uu_HAL/python-dateutil-1.5/setup.cfg
    Running python-dateutil-1.5/setup.py -q bdist_egg --dist-dir /tmp/easy_install-Uu_HAL/python-dateutil-1.5/egg-dist-tmp-fCQCzQ

    Installed /tmp/protobuf/build/protobuf/python_dateutil-1.5-py2.7.egg

Requirement already satisfied (use --upgrade to upgrade): setuptools in /tmp/protobuf/lib/python2.7/site-packages (from protobuf)
Installing collected packages: protobuf
Running setup.py install for protobuf
    Skipping installation of /tmp/protobuf/lib/python2.7/site-packages/google/__init__.py (namespace package)

    Installing /tmp/protobuf/lib/python2.7/site-packages/protobuf-2.6.0-py2.7-nspkg.pth
Successfully installed protobuf
Cleaning up...
$ /tmp/protobuf/bin/python
Python 2.7.6 (default, Nov 15 2013, 11:35:43) 
[GCC 4.6.3] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import google.protobuf
>>> 

[jgeewax]

I copied and pasted the output of installing on Ubuntu 13.10 (no virtual environment which unfortunately is how many of our users will install this...). I think the best bet is to do protobuf >= 2.5.0...

[dhermes]

So the versions in the virtualenv output from @tseaver are:

protobuf-2.6.0.tar.gz
google_apputils-0.4.0-py2.7.egg
pytz-2014.7-py2.7.egg
python_gflags-2.0-py2.7.egg
python_dateutil-1.5-py2.7.egg

So what is happening is that setuptools respects python-dateutil>=1.4,<2 but aptitude / apt-get installs Python dependencies in a different way.

It seems the issue happens in the python-google-apputils package:
http://packages.ubuntu.com/trusty/python/python-google-apputils

That is the only hard dependency of the protobuf package:
http://anonscm.debian.org/cgit/collab-maint/protobuf.git/tree/debian/control#n19

UPDATE: I completely wiped all my installed dependencies on Ubuntu 14.04 and apt-get install python-protobuf worked just fine. I think output from an already sunset version (13.10) is not so relevant to us. However, this didn't actually install the dateutil dependency so I'm now feeling like

[jgeewax]

Making our code work with protobuf 2.5 and 2.6 isn't a bad thing IMO. I've updated the path with two things:

1. setup.py: protobut >= 2.5.0
2. helpers.py: assumes v 2.5.0 (which doesn't return the coerced value), but works fine with 2.6.0 also

Problem solved?

[dhermes]

Yes, it seems in protocolbuffers/protobuf#36 that the install issue will "go away" very shortly so IMO this is enough. LGTM (pending my tiny "typecast" comment).