fix: Add compatibility with protobuf==5.x #433
Draft
+342
−52
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
parthea
force-pushed
the
add-protobuf-5-compatibility
branch
10 times, most recently
from
8ea8426
to
39e4d33
Compare
parthea
changed the title
parthea
force-pushed
the
add-protobuf-5-compatibility
branch
from
39e4d33
to
62b37ee
Compare
parthea
changed the title
parthea
force-pushed
the
add-protobuf-5-compatibility
branch
2 times, most recently
from
259b2a4
to
19c487f
Compare
parthea
added
the
owlbot:run
gcf-owl-bot
bot
removed
the
owlbot:run
vchudnov-g
reviewed
Mar 19, 2024
proto/message.py
Outdated
| including_default_value_fields (Optional(bool)): Deprecated. Use argument | ||
| `always_print_fields_with_no_presence` instead. An option that | ||
| determines whether the default field values should be included in the results. | ||
| Default is True. If `always_print_fields_with_no_presence` is set to False, this |
There was a problem hiding this comment.
Both of these fields say that they don't take effect if the other is False. This is confusing enough that it's not obvious what the right semantics are and whether they make sense.
I recommend setting the default value for both fields in the arg list above to be None, and then using them thus:
if ((always_print_fields_with_no_presence is not None) and
(including_default_value_fields is not None) and
(always_print_fields_with_no_presence != including_default_value_fields):
raise ValueError("new and deprecated parameters are inconsistent")
print_field = (always_print_fields_with_no_presence if always_print_fields_with_no_presence is not None
else (including_default_value_fields if including_default_value_fields is not None
else True)
and simplifying the comments for both parameters to say that any value explicitly provided for one must be consistent with the any value explicitly provided for the other.
proto/message.py
Outdated
| return MessageToJson( | ||
| cls.pb(instance), | ||
| use_integers_for_enums=use_integers_for_enums, | ||
| including_default_value_fields=including_default_value_fields, |
There was a problem hiding this comment.
If we're deprecating this field on thisA PI surface, we should have the same deprecation logic on the RHS here as in the else branch (and I suggest the print_field variable I defined in my other comment).
| @@ -448,24 +483,49 @@ def to_dict( | |||
| preserving_proto_field_name (Optional(bool)): An option that | |||
| determines whether field name representations preserve | |||
| proto case (snake_case) or use lowerCamelCase. Default is True. | |||
| including_default_value_fields (Optional(bool)): An option that | |||
| including_default_value_fields (Optional(bool)): Deprecated. Use argument | |||
There was a problem hiding this comment.
same comments as in to_json
parthea
changed the title
|
The pre-release failure in https://github.com/googleapis/proto-plus-python/actions/runs/8758049270/job/24037997447 is captured in #444 |
parthea
changed the title
parthea
added
the
owlbot:run
gcf-owl-bot
bot
removed
the
owlbot:run
…Python object in tests
parthea
force-pushed
the
add-protobuf-5-compatibility
branch
from
327fb52
to
09b9bc4
Compare
parthea
added
the
owlbot:run
gcf-owl-bot
bot
removed
the
owlbot:run
parthea
commented
May 13, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
BEGIN_COMMIT_OVERRIDE
feat: Add
always_print_fields_with_no_presencefields toto_jsonandto_dictdocs: Deprecate field
including_default_value_fieldsinto_jsonandto_dictfix: Add compatibility with protobuf==5.x
fix: AttributeError module 'google._upb._message' has no attribute 'MessageMapContainer'
ci: Add support for pre-release sessions
ci: Add python 3.13 to testing matrix
ci: Resolve
RecursionError: maximum recursion depth exceeded while calling a Python objectwhen running testsEND_COMMIT_OVERRIDE
Fixes #425
Fixes #427
Fixes #431
Fixes #445
For compatibility with https://pypi.org/project/protobuf/5.26.0rc2/
See protocolbuffers/protobuf@2699579