Fix error with setting repeated proto field for bundling.

.gitignore

@@ -65,5 +65,8 @@ coverage.xml
 docs/_build/
 _build
 
+#Protobuf
+*_pb2.py
+
 # PyBuilder
 target/

.travis.yml

@@ -1,4 +1,13 @@
 sudo: false
+env:
+  - BREW_HOME=$HOME/.linuxbrew
+
+# Protobuf is very expensive to install, so we cache it between builds
+before_install: ./install-protobuf3.sh
+cache:
+  directories:
+    - $BREW_HOME
+
 language: python
 python:
   - "2.7"

google/gax/__init__.py

@@ -33,7 +33,7 @@
 import collections
 
 
-__version__ = '0.12.0'
+__version__ = '0.12.1'
 
 
 INITIAL_PAGE = object()

google/gax/bundling.py

@@ -67,11 +67,9 @@ def _str_dotted_getattr(obj, name):
     Raises:
        AttributeError if the named attribute does not exist.
     """
-    if name.find('.') == -1:
-        return getattr(obj, name)
     for part in name.split('.'):
         obj = getattr(obj, part)
-    return str(obj) if obj is not None else None
+    return str(obj) if obj else None
 
 
 def compute_bundle_id(obj, discriminator_fields):
@@ -190,7 +188,9 @@ def _run_with_subresponses(self, req, subresponse_field, kwargs):
                 for i, event in zip(in_sizes, self._event_deque):
                     next_copy = copy.copy(resp)
                     subresponses = all_subresponses[start:start + i]
-                    setattr(next_copy, subresponse_field, subresponses)
+                    # TODO: assumes we are using GRPC; abstract into config.py for portability
+                    next_copy.ClearField(subresponse_field)
+                    getattr(next_copy, subresponse_field).extend(subresponses)
                     start += i
                     event.result = next_copy
                     event.set()

install-protobuf3.sh

@@ -0,0 +1,22 @@
+#!/bin/bash
+
+export PATH="$BREW_HOME/bin:$PATH"
+
+echo "Testing for protoc command"
+command -v protoc || {
+  echo "Testing for brew command"
+  command -v brew || {
+    echo "Installing brew"
+    # install brew in headless mode to avoid user prompt requiring input
+    </dev/null ruby -e \
+      "$(curl -fsSL https://raw.githubusercontent.com/Linuxbrew/install/master/install)"
+  }
+
+  echo "Installing proto3 via brew"
+  brew install --devel protobuf
+}
+
+# ensure protoc is up-to-date in our cache
+echo "Upgrading brew packages"
+brew update
+brew upgrade

test/fixture.proto

@@ -0,0 +1,30 @@
+// Copyright 2016 Google Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+syntax = "proto3";
+
+package google.protobuf;
+
+message Simple {
+  string field1 = 1;
+  string field2 = 2;
+}
+
+message Outer {
+  Simple inner = 1;
+  string field1 = 2;
+}
+
+message Bundled {
+  repeated string field1 = 1;
+}

test/test_bundling.py

@@ -37,25 +37,22 @@
 
 from google.gax import bundling, BundleOptions, BundleDescriptor
 
+from fixture_pb2 import Simple, Outer, Bundled
 
-# pylint: disable=too-few-public-methods
-class _Simple(object):
-    def __init__(self, value, other_value=None):
-        self.field1 = value
-        self.field2 = other_value
 
-    def __eq__(self, other):
-        return (self.field1 == other.field1 and
-                self.field2 == other.field2)
+def _Simple(value, other_value=None):
+    if other_value is None:
+        return Simple(field1=value)
+    else:
+        return Simple(field1=value, field2=other_value)
 
-    def __str__(self):
-        return 'field1={0}, field2={1}'.format(self.field1, self.field2)
 
+def _Outer(value):
+    return Outer(inner=_Simple(value, value), field1=value)
 
-class _Outer(object):
-    def __init__(self, value):
-        self.inner = _Simple(value, other_value=value)
-        self.field1 = value
+
+def _Bundled(value):
+    return Bundled(field1=value)
 
 
 class TestComputeBundleId(unittest2.TestCase):
@@ -135,7 +132,7 @@ def _make_a_test_task(api_call=_return_request):
         api_call,
         'an_id',
         'field1',
-        _Simple([]),
+        _Bundled([]),
         dict())
 
 
@@ -212,13 +209,13 @@ def test_run_sends_the_bundle_elements(self):
                 'message': 'a single bundled message',
                 'has_event': True,
                 'count_before_run': 1,
-                'want': _Simple([simple_msg])
+                'want': _Bundled([simple_msg])
             }, {
                 'update': (lambda t: _extend_with_n_elts(t, simple_msg, 5)),
                 'message': '5 bundle messages',
                 'has_event': True,
                 'count_before_run': 5,
-                'want': _Simple([simple_msg] * 5)
+                'want': _Bundled([simple_msg] * 5)
             }
         ]
         for t in tests:
@@ -259,7 +256,7 @@ def test_calling_the_canceller_stops_the_element_from_getting_sent(self):
         self.assertEquals(test_task.element_count, 1)
         test_task.run()
         self.assertEquals(test_task.element_count, 0)
-        self.assertEquals(_Simple([another_msg]), another_event.result)
+        self.assertEquals(_Bundled([another_msg]), another_event.result)
         self.assertFalse(an_event.is_set())
         self.assertIsNone(an_event.result)
 
@@ -283,7 +280,7 @@ def test_api_calls_are_grouped_by_bundle_id(self):
                     api_call,
                     an_id,
                     SIMPLE_DESCRIPTOR,
-                    _Simple([an_elt])
+                    _Bundled([an_elt])
                 )
                 self.assertIsNotNone(
                     got_event.canceller,
@@ -297,14 +294,14 @@ def test_api_calls_are_grouped_by_bundle_id(self):
                 api_call,
                 an_id,
                 SIMPLE_DESCRIPTOR,
-                _Simple([an_elt])
+                _Bundled([an_elt])
             )
             self.assertIsNotNone(got_event.canceller,
                                  'missing expected canceller')
             self.assertTrue(
                 got_event.is_set(),
                 'event is not set after triggering element')
-            self.assertEquals(_Simple([an_elt] * threshold),
+            self.assertEquals(_Bundled([an_elt] * threshold),
                               got_event.result)
 
     def test_each_event_has_exception_when_demuxed_api_call_fails(self):
@@ -320,7 +317,7 @@ def test_each_event_has_exception_when_demuxed_api_call_fails(self):
                 api_call,
                 bundle_id,
                 DEMUX_DESCRIPTOR,
-                _Simple(['%s%d' % (an_elt, i)])
+                _Bundled(['%s%d' % (an_elt, i)])
             )
             self.assertFalse(
                 got_event.is_set(),
@@ -331,7 +328,7 @@ def test_each_event_has_exception_when_demuxed_api_call_fails(self):
             api_call,
             bundle_id,
             DEMUX_DESCRIPTOR,
-            _Simple(['%s%d' % (an_elt, threshold - 1)])
+            _Bundled(['%s%d' % (an_elt, threshold - 1)])
         )
         events.append(last_event)
 
@@ -359,7 +356,7 @@ def test_each_event_has_its_result_from_a_demuxed_api_call(self):
                 api_call,
                 bundle_id,
                 DEMUX_DESCRIPTOR,
-                _Simple(['%s%d' % (an_elt, i)] * i)
+                _Bundled(['%s%d' % (an_elt, i)] * i)
             )
             events.append(got_event)
         previous_event = None
@@ -370,12 +367,12 @@ def test_each_event_has_its_result_from_a_demuxed_api_call(self):
             self.assertTrue(event.is_set(),
                             'event is not set after triggering element')
             self.assertEquals(event.result,
-                              _Simple(['%s%d' % (an_elt, index)] * index))
+                              _Bundled(['%s%d' % (an_elt, index)] * index))
             previous_event = event
 
     def test_each_event_has_same_result_from_mismatched_demuxed_api_call(self):
         an_elt = 'dummy message'
-        mismatched_result = _Simple([an_elt, an_elt])
+        mismatched_result = _Bundled([an_elt, an_elt])
         bundle_id = 'an_id'
         threshold = 5  # arbitrary, greater than 1
         options = BundleOptions(element_count_threshold=threshold)
@@ -388,7 +385,7 @@ def test_each_event_has_same_result_from_mismatched_demuxed_api_call(self):
                 lambda x: mismatched_result,
                 bundle_id,
                 DEMUX_DESCRIPTOR,
-                _Simple(['%s%d' % (an_elt, i)] * i)
+                _Bundled(['%s%d' % (an_elt, i)] * i)
             )
             events.append(got_event)
         previous_event = None
@@ -409,7 +406,7 @@ def test_schedule_passes_kwargs(self):
             _return_kwargs,
             bundle_id,
             SIMPLE_DESCRIPTOR,
-            _Simple([an_elt]),
+            _Bundled([an_elt]),
             {'an_option': 'a_value'}
         )
         self.assertEquals('a_value',
@@ -430,7 +427,7 @@ def test_api_call_not_invoked_until_threshold(self):
                 api_call,
                 an_id,
                 SIMPLE_DESCRIPTOR,
-                _Simple([an_elt])
+                _Bundled([an_elt])
             )
             self.assertIsNotNone(
                 got_event.canceller,
@@ -440,7 +437,7 @@ def test_api_call_not_invoked_until_threshold(self):
                 self.assertIsNone(got_event.result)
             else:
                 self.assertTrue(got_event.is_set())
-                self.assertEquals(_Simple([an_elt] * threshold),
+                self.assertEquals(_Bundled([an_elt] * threshold),
                                   got_event.result)
 
 
@@ -459,7 +456,7 @@ def test_api_call_not_invoked_until_threshold(self):
                 api_call,
                 an_id,
                 SIMPLE_DESCRIPTOR,
-                _Simple([an_elt])
+                _Bundled([an_elt])
             )
             self.assertIsNotNone(
                 got_event.canceller,
@@ -469,7 +466,7 @@ def test_api_call_not_invoked_until_threshold(self):
                 self.assertIsNone(got_event.result)
             else:
                 self.assertTrue(got_event.is_set())
-                self.assertEquals(_Simple([an_elt] * elts_for_threshold),
+                self.assertEquals(_Bundled([an_elt] * elts_for_threshold),
                                   got_event.result)
 
 
@@ -487,7 +484,7 @@ def test_api_call_is_scheduled_on_timer(self, timer_class):
             api_call,
             an_id,
             SIMPLE_DESCRIPTOR,
-            _Simple([an_elt])
+            _Bundled([an_elt])
         )
         self.assertIsNotNone(got_event, 'missing event after first request')
         self.assertIsNone(got_event.result)

tox.ini

@@ -6,31 +6,35 @@ envlist = py27,pep8,pylint-errors,pylint-full
 
 [testenv]
 setenv =
-       PYTHONPATH = {toxinidir}
+       PYTHONPATH = {toxinidir}:{toxinidir}/src-gen/test
 
 deps = -r{toxinidir}/test-requirements.txt
        -r{toxinidir}/requirements.txt
-commands = py.test --timeout=30 --cov-report html --cov-report=term --cov {toxinidir}/google
+whitelist_externals = mkdir
+                      protoc
+commands = -mkdir src-gen
+           -{env:BREW_HOME}/bin/protoc --python_out=src-gen test/fixture.proto
+           -py.test --timeout=30 --cov-report html --cov-report=term --cov {toxinidir}/google
 
 [testenv:pep8]
 deps = flake8
-commands = flake8 --max-complexity=10 google test --ignore=E501
+commands = flake8 --max-complexity=10 google test --ignore=E501 --exclude=src-gen
 
 [testenv:pylint-errors]
 deps = pylint
        -r{toxinidir}/test-requirements.txt
        -r{toxinidir}/requirements.txt
-commands = pylint -f colorized -E google test
+commands = pylint -f colorized -E google test --ignore=src-gen
 
 [testenv:pylint-warnings]
 deps = pylint
-commands = pylint -f colorized -d all -e W -r n google test
+commands = pylint -f colorized -d all -e W -r n google test --ignore=src-gen
 
 [testenv:pylint-full]
 deps = pylint
        -r{toxinidir}/test-requirements.txt
        -r{toxinidir}/requirements.txt
-commands = pylint -f colorized -e E,W,R -d fixme,locally-disabled google test
+commands = pylint -f colorized -e E,W,R -d fixme,locally-disabled google test --ignore=src-gen
 
 [testenv:devenv]
 commands =