Skip to content

Conversation

@software-dov
Copy link
Contributor

Fields can customize their json name representation.
Add a test to verify that this customization is respected by both
Python and C++ runtimes.
Boost protobuf requirement to gain required upstream bugfix.

@software-dov software-dov requested review from a team, busunkim96 and parthea October 26, 2021 21:15
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Oct 26, 2021
@codecov
Copy link

codecov bot commented Oct 26, 2021

Codecov Report

Merging #270 (c6f87f4) into master (6a43682) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #270   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           20        22    +2     
  Lines          862       961   +99     
  Branches       149       170   +21     
=========================================
+ Hits           862       961   +99     
Impacted Files Coverage Δ
proto/modules.py 100.00% <ø> (ø)
proto/enums.py 100.00% <100.00%> (ø)
proto/fields.py 100.00% <100.00%> (ø)
proto/marshal/collections/repeated.py 100.00% <100.00%> (ø)
proto/marshal/marshal.py 100.00% <100.00%> (ø)
proto/marshal/rules/bytes.py 100.00% <100.00%> (ø)
proto/marshal/rules/message.py 100.00% <100.00%> (ø)
proto/marshal/rules/stringy_numbers.py 100.00% <100.00%> (ø)
proto/message.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 79ec820...c6f87f4. Read the comment docs.

Fields can customize their json name representation.
Add a test to verify that this customization is respected by both
Python and C++ runtimes.
Boost protobuf requirement to gain required upstream bugfix.
@software-dov software-dov merged commit 2bdbcce into googleapis:master Oct 27, 2021
#
# e.g., if setup.py has "foo >= 1.14.0, < 2.0.0dev",
# Then this file should have foo==1.14.0
protobuf==3.15.6
Copy link
Contributor

Choose a reason for hiding this comment

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

This should have updated to the minimum supported version (so that we continue to support it even when future versions are released).

Copy link
Contributor

Choose a reason for hiding this comment

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

See #273.

platforms="Posix; MacOS X",
include_package_data=True,
install_requires=("protobuf >= 3.12.0",),
install_requires=("protobuf >= 3.19.0",),
Copy link

Choose a reason for hiding this comment

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

This should not have been included in a patch release. It's causing problems over in the conda ecosystem, where the version of libprotobuf is a bit slower to update as it require recompilation of many many packages. Currently the latest grcp-cpp package is compiled with libprotobuf==3.18.1.

Copy link

Choose a reason for hiding this comment

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

Update: literally today is when they are rolling out changes to the build environment to support protobuf 3.19 conda-forge/grpc-cpp-feedstock#124

Copy link

Choose a reason for hiding this comment

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

For future reference, this is the where pin for conda was updated: conda-forge/conda-forge-pinning-feedstock#2049

I recommend we don't update minimum protobuf beyond this in future

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we need to add conda tests to verify that installation is possible?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

json_name is not respected for proto.Field objects using Python proto impl

4 participants