Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add preserving_proto_field_name option to to_dict methods #166

Closed
tswast opened this issue Dec 1, 2020 · 5 comments · Fixed by #211
Closed

add preserving_proto_field_name option to to_dict methods #166

tswast opened this issue Dec 1, 2020 · 5 comments · Fixed by #211
Labels
P2 A nice-to-fix bug priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@tswast
Copy link

tswast commented Dec 1, 2020

MessageToDict provides a preserving_proto_field_name option to either preserve proto (snake_case) names or convert to lowerCamelCase.

This is blocking google-cloud-bigquery migration to proto-plus classes googleapis/python-bigquery#319 as the existing client is relying on the default protobuf behavior (converts to lowerCamelCase)

@yoshi-automation yoshi-automation added the triage me I really want to be triaged. label Dec 2, 2020
@software-dov software-dov added P2 A nice-to-fix bug priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. and removed triage me I really want to be triaged. labels Dec 2, 2020
@software-dov
Copy link
Contributor

To clarify: is the client doing proto->dict conversion, or is it something the users may do and want to retain compatibility? Are there specific code paths or other features that are breaking, or is it more of a testing-headache fix?

@tswast
Copy link
Author

tswast commented Dec 2, 2020

More the latter "testing-headache fix". It was too much for our vendor to handle because there were more places than I realized that assume lowerCamelCase.

@tswast
Copy link
Author

tswast commented Dec 2, 2020

We do expose the JSON representation via a to_api_repr method (which was requested by some customers for archiving purposes). I suppose the switch to snake case could be considered a breaking change there.

@software-dov
Copy link
Contributor

I think this may be blocked by a protobuf bug that does not respect JSON field name specification: protocolbuffers/protobuf#7910
I'm not sure about that though.

@mik-laj
Copy link

mik-laj commented Dec 28, 2020

This would also be a great help for Apache Airflow (Cloud Composer) as we could avoid breaking changes that affect large numbers of users.
Our use case is described in our documentation:
https://github.com/apache/airflow/blob/master/airflow/providers/google/ADDITIONAL_INFO.md
Our plans to migrate to new libraries are described in the tickets, but they are not very positive for users:
apache/airflow#13230
apache/airflow#12116

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 A nice-to-fix bug priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
4 participants