Skip to content
This repository has been archived by the owner on Mar 20, 2018. It is now read-only.

Fix error with setting repeated proto field for bundling. #111

Merged
merged 19 commits into from
Jun 22, 2016

Conversation

bjwatson
Copy link
Contributor

@bjwatson bjwatson commented Jun 22, 2016

Update bundling tests to use actual proto messages, to prove this fix
works.

Fixes #110

@bjwatson
Copy link
Contributor Author

@tbetbetbe I know that the pep8 and pylint tests currently fail. They just need to ignore the generated fixture_pb2.py file. Otherwise this is ready to review.

Update bundling tests to use actual proto messages, to prove this fix
works.

[testenv:pylint-errors]
setenv =
PYTHONPATH = {toxinidir}:{toxinidir}/test
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this because we weren't linting the test/ directory previously?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This wasn't really needed. I got rid of this change.

@codecov-io
Copy link

codecov-io commented Jun 22, 2016

Current coverage is 97.16%

Merging #111 into master will decrease coverage by <.01%

@@             master       #111   diff @@
==========================================
  Files             8          8          
  Lines           600        599     -1   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits            583        582     -1   
  Misses           17         17          
  Partials          0          0          

Powered by Codecov. Last updated by 8c26941...8826e7f

@bjwatson
Copy link
Contributor Author

Woot! Woot! I finally got a passing build with protoc / Travis CI / tox. :)

@geigerj
Copy link
Contributor

geigerj commented Jun 22, 2016

LGTM

@geigerj
Copy link
Contributor

geigerj commented Jun 22, 2016

Note: this adds an implicit dependency of bundling.py on proto, namely the ClearField method. Ideally this should be moved into gRPC.py and called through the config.py "interface", but that can be done as a follow-up.

@bjwatson
Copy link
Contributor Author

Added TODO comment to code based on feedback

@bjwatson bjwatson merged commit a5e1944 into googleapis:master Jun 22, 2016
@bjwatson bjwatson deleted the issue-110 branch June 22, 2016 22:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bundling sub-response handling is broken for actual proto objects
4 participants