fix: add support for protobuf 5.x by parthea · Pull Request #644 · googleapis/python-api-core

parthea · 2024-04-27T13:20:27Z

Fixes#628 🦕
Fixes#641 🦕
Fixes#642 🦕

Regarding the including_default_value_fields argument of json_format.MessageToDict() in protobuf:

In protobuf version 3.19.6 - The default value of including_default_value_fields was False
https://github.com/protocolbuffers/protobuf/blob/5cba162a5d93f8df786d828621019e03e50edb4f/python/google/protobuf/json_format.py#L92

At the time that the argument was removed in protobuf 5.x - the default value of including_default_value_fields was False
protocolbuffers/protobuf@2699579#diff-8de817c14d6a087981503c9aea38730b1b3e98f4e306db5ff9d525c7c304f234

IOW, setting argument including_default_value_fields to False had no effect as it was the default behaviour.

The Unit tests / unit_prerelease-3.12 presubmit check was added to run tests against the latest pre-release version of protobuf as several dependencies have a constraint on protobuf<5

anndro · 2024-05-16T08:53:33Z

Hello @parthea is there any scheduled date for release this pull request?

zhangskz · 2024-05-30T16:36:03Z

@parthea Is there anything blocking this PR still?

vchudnov-g

A couple of suggestions, and some questions, but nothing major. This looks good!

vchudnov-g · 2024-05-31T17:54:09Z

+ # For backwards compatibility with protobuf 3.x 4.x
+ # Remove once support for protobuf 3.x and 4.x is dropped
+ # https://github.com/googleapis/python-api-core/issues/643
+ if PROTOBUF_VERSION[0:2] in ["3.", "4."]:
+ request_kwargs = json_format.MessageToDict(
+ request,
+ preserving_proto_field_name=True,
+ including_default_value_fields=True, # type: ignore # backward compatibility
+ )
+ else:
+ request_kwargs = json_format.MessageToDict(
+ request,
+ preserving_proto_field_name=True,
+ always_print_fields_with_no_presence=True,
+ )
+
 transcoded_request = path_template.transcode(http_options, **request_kwargs)


You have this block repeated in four places. I suggest factoring it out into a helper function that only needs to take one parameter:
Suggested change
# For backwards compatibility with protobuf 3.x 4.x
# Remove once support for protobuf 3.x and 4.x is dropped
# https://github.com/googleapis/python-api-core/issues/643
ifPROTOBUF_VERSION[0:2] in ["3.", "4."]:
request_kwargs=json_format.MessageToDict(
request,
preserving_proto_field_name=True,
including_default_value_fields=True, # type: ignore # backward compatibility
)
else:
request_kwargs=json_format.MessageToDict(
request,
preserving_proto_field_name=True,
always_print_fields_with_no_presence=True,
)
transcoded_request=path_template.transcode(http_options, **request_kwargs)
transcoded_request=_transcode_request(request)

Fixed in 5a49a53

vchudnov-g · 2024-05-31T18:11:10Z

+
+ if prerelease:
+ install_prerelease_dependencies(
+ session, f"{constraints_dir}/constraints-{PYTHON_VERSIONS[0]}.txt"


I'm confused: in line 206, where we call default(..., prerelease=True), the decorator constrains python=PYTHON_VERSIONS[-1]. But here we are referencing the constraints for PYTHON_VERSIONS[0]. Could you clarify?

As per the comment in below, the constraints file for the lowest supported python version contains a list of all of the dependencies of the library. The function install_prerelease_dependencies will extract the dependencies from the file.
python-api-core/testing/constraints-3.7.txt
Lines 1 to 7 in 126b5c7
# This constraints file is used to check that lower bounds
# are correct in setup.py
# List *all* library dependencies and extras in this file.
# Pin the version to the lower bound.
#
# e.g., if setup.py has "foo >= 1.14.0, < 2.0.0dev",
# Then this file should have foo==1.14.0
python-api-core/noxfile.py
Lines 79 to 88 in 7fbce0d
definstall_prerelease_dependencies(session, constraints_path):
withopen(constraints_path, encoding="utf-8") asconstraints_file:
constraints_text=constraints_file.read()
# Ignore leading whitespace and comment lines.
constraints_deps= [
match.group(1)
formatchinre.finditer(
r"^\s*(\S+)(?===\S+)", constraints_text, flags=re.MULTILINE
)
]
Once we have a list of dependencies, we will install them independently with the --pre and --no-deps option. `--upgrade is also added to ensure that we get the latest pre-release.
python-api-core/noxfile.py
Lines 99 to 100 in 7fbce0d
fordepinprerel_deps:
session.install("--pre", "--no-deps", "--upgrade", dep)

vchudnov-g · 2024-05-31T18:15:54Z



-def default(session, install_grpc=True):
+def install_prerelease_dependencies(session, constraints_path):


Sorry, I have gaps in my mental model for testing prerelease;
This installs our dependencies at pre-release versions, right?
Is the only difference, then, the use of --pre below? If it's the same set of dependencies, it seems it would be clearer to have them loaded in the same place for pre-release and stable versions, and add the extra install parameters conditionally on each thing we install.
(It might be easier to talk synchronously about this.)

There is a difference in that we don't allow transitive dependencies in the install_prerelease_dependencies session as we want to ensure that we're installing the pre-release version of each dependency. In the non-prerelease session, we are installing transitive dependencies.

vchudnov-g · 2024-05-31T18:25:32Z


-@nox.session(python=["3.7", "3.8", "3.9", "3.10", "3.11", "3.12"])
+@nox.session(python=PYTHON_VERSIONS[-1])
+def unit_prerelease(session):


So this tests running against the pre-release versions of the dependencies, right? And we only bother to do this with the latest Python run-time.
A comment might be helpful. Also, my personal preference would to rename this (and similar code elsewhere) as unit_with_prerelease_deps. Every time I read something like unit_prerelease my mind first assumes we're testing the pre-release version of this library, not of its dependencies.

Fixed in e1efa83

vchudnov-g · 2024-05-31T18:48:55Z

 strategy:
 matrix:
- option: ["", "_grpc_gcp", "_wo_grpc"]
+ option: ["", "_grpc_gcp", "_wo_grpc", "_prerelease"]


I think this more succinct code should work (I haven't tested it) and would be easier to maintain.
Ref: https://docs.github.com/en/actions/using-jobs/using-a-matrix-for-your-jobs#expanding-or-adding-matrix-configurations
option: ["", "_grpc_gcp", "_wo_grpc"] python: -"3.7"-"3.8"-"3.9"-"3.10"-"3.11"-"3.12"exclude: -option: "_wo_grpc"python: 3.7-option: "_wo_grpc"python: 3.8-option: "_wo_grpc"python: 3.9include: -option: "_prerelease"python: 3.12

This comment is obsolete with the changes in e1efa83

Co-authored-by: Victor Chudnovsky <vchudnov@google.com>

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

…gleapis/python-api-core into add-support-for-protobuf-5-x

parthea · 2024-06-11T20:07:10Z

@ohmayr PTAL

ohmayr · 2024-06-12T16:56:41Z

+from google.api_core.operations_v1.abstract_operations_client import (
+ AbstractOperationsClient,
+)


Why is this file changed?

I ran black on all of the files and this file was formatted

ohmayr · 2024-06-12T17:06:01Z



-@nox.session(python="3.8")
+@nox.session(python=DEFAULT_PYTHON_VERSION)


Just to clarify, do we want to run this against all versions?

We normally only run it on 1 version but we can expand it if there is value

ohmayr · 2024-06-12T17:24:23Z

- including_default_value_fields=False,
 preserving_proto_field_name=False,
 use_integers_for_enums=False,
 )


This is essentially also converting a message to a dict. So we could use a similar approach to what we're doing for request_params and define a helper function so we're consistent. But it's more of a preference and could be a follow up.
I'll leave it up to you since this isn't a blocker!

I took a closer look at making this change and I don't believe the helper function will result in fewer lines of code. The reason that we had the helper function elsewhere was to remove duplication for the protobuf 3.x/4.x compatibility code but we don't have the duplication here.

JackCloudman · 2024-06-17T06:19:54Z

Hello, I am eagerly anticipating the next release! Could you please provide any updates on the release date? Thank the whole team for your hard work and dedication.

product-auto-labelBot added the size: m Pull request size is medium. label Apr 27, 2024

parthea force-pushed the add-support-for-protobuf-5-x branch 7 times, most recently from f8f719b to a650198Compare April 27, 2024 15:11

fix: add support for protobuf 5.x
ac9cad8

parthea force-pushed the add-support-for-protobuf-5-x branch from a650198 to ac9cad8Compare April 27, 2024 15:12

Merge branch 'main' into add-support-for-protobuf-5-x
a62ea6e

parthea marked this pull request as ready for review April 29, 2024 14:36

parthea requested review from a teamApril 29, 2024 14:36

blunderbuss-gcfBot assigned ohmayr Apr 29, 2024

remove pin for types-protobuf
13e1567

parthea added the owlbot:run Add this label to trigger the Owlbot post processor. label Apr 29, 2024

gcf-owl-botBot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Apr 29, 2024

remove pytest from noxfile
8b6bc48

olivi-eh mentioned this pull request May 21, 2024
chore(deps): update dependency requests to v2.32.0 [security] - autoclosed GoogleCloudPlatform/microservices-demo#2557
Closed
1 task

Merge branch 'main' into add-support-for-protobuf-5-x
583b8ce

parthea assigned vchudnov-g May 30, 2024

parthea requested review from ohmayr and vchudnov-g May 30, 2024 17:24

vchudnov-g reviewed May 31, 2024
View reviewed changes

parthea assigned parthea and unassigned vchudnov-g and ohmayr Jun 4, 2024

parthea mentioned this pull request Jun 4, 2024
chore(main): release 2.19.1 #650
Merged

veblush mentioned this pull request Jun 4, 2024
[Dep] Upgrading Protobuf to v27.2 grpc/grpc#36753
Closed

partheaand others added 8 commits June 11, 2024 19:40

refactor common code
5a49a53

Refactor
3bca72c
Co-authored-by: Victor Chudnovsky <vchudnov@google.com>

run black
f263d69

🦉 Updates from OwlBot post-processor
2396495
See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

Merge branch 'add-support-for-protobuf-5-x' ofhttps://github.com/goo…
ef6050f
…gleapis/python-api-core into add-support-for-protobuf-5-x

mypy
7fbce0d

run pre-release test against all python versions
e1efa83

filter warning
28a41f8

parthea assigned ohmayr and unassigned parthea Jun 11, 2024

ohmayr approved these changes Jun 12, 2024
View reviewed changes

parthea assigned parthea and unassigned ohmayr Jun 19, 2024

Merge branch 'main' into add-support-for-protobuf-5-x
0037d7a

parthea enabled auto-merge (squash) June 19, 2024 15:21

parthea merged commit fda0ca6 into mainJun 19, 2024

parthea deleted the add-support-for-protobuf-5-x branch June 19, 2024 15:22

parthea mentioned this pull request Jun 19, 2024
chore: Release candidate v2.19.1rc0 #672
Closed

This was referenced May 30, 2025
chore(main): release 2.0.0 #822
Closed
chore(main): release 2.0.0 #823
Closed

	# This constraints file is used to check that lower bounds
	# are correct in setup.py
	# List all library dependencies and extras in this file.
	# Pin the version to the lower bound.
	#
	# e.g., if setup.py has "foo >= 1.14.0, < 2.0.0dev",
	# Then this file should have foo==1.14.0

	definstall_prerelease_dependencies(session, constraints_path):
	withopen(constraints_path, encoding="utf-8") asconstraints_file:
	constraints_text=constraints_file.read()
	# Ignore leading whitespace and comment lines.
	constraints_deps= [
	match.group(1)
	formatchinre.finditer(
	r"^\s*(\S+)(?===\S+)", constraints_text, flags=re.MULTILINE
	)
	]

	fordepinprerel_deps:
	session.install("--pre", "--no-deps", "--upgrade", dep)



		def default(session, install_grpc=True):
		def install_prerelease_dependencies(session, constraints_path):



		@nox.session(python="3.8")
		@nox.session(python=DEFAULT_PYTHON_VERSION)

Uh oh!

Conversation

parthea commented Apr 27, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

anndro commented May 16, 2024

Uh oh!

zhangskz commented May 30, 2024

Uh oh!

vchudnov-g left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

parthea commented Jun 11, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JackCloudman commented Jun 17, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

parthea commented Apr 27, 2024•
edited
Loading