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 protoc Param --bq-schema_opt=type-override=field_name.field_type #41

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

its28604
Copy link

Base on the --bq-schema_opt=single-message, add additional --bq-schema_opt=type-override=field_name.field_type to tell protoc-gen-bq-schema to specify certain field's TypeOverride field options.

If the single-message doesn't be specify, this will not work as well.
By add this param, can minimize the use of imports.

The primary motivation for this feature is to facilitate the maintenance of consistent Pub/Sub Schemas and BigQuery schemas using the same .proto files. In certain scenarios, such as when using .proto files to create Pub/Sub Schemas, importing additional files (like bq_field.proto) is not permissible. This enhancement circumvents the need for such imports, simplifying the workflow and making schema management more streamlined, especially for users who need to maintain compatibility across different systems using Protocol Buffers.

Copy link

google-cla bot commented Dec 21, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@yugui
Copy link
Collaborator

yugui commented Dec 30, 2023

Thank you for your contribution, @its28604 . I like the idea of type overriding in options.

I would like to discuss the details of the feature design a little more before further reviews on the implementation.

IIUC, the feature of type overriding is independent from single-message. Therefore, we might want to specify the fully qualified field name rather than the local field name within a single message.
For the same reason, we might want to keep the option parsing independent from the one for single-message.

I would like to hear your opinion.

@its28604
Copy link
Author

its28604 commented Jan 1, 2024

Hi @yugui ,

I understand that this feature might be independent rather than a local feature within single-message. However, I placed it within single-message because, in my opinion, the type override in options is only necessary when using the single-message feature. In other scenarios, it's simpler to add the parameter in the field options. Nonetheless, making the feature independent is also an excellent idea, as it would indeed increase the generality of this feature.

I appreciate the exchange of ideas, and please let me know if your perspective is different.

@anthonyalayo
Copy link

I was just looking for a feature like this, and saw the exchange. I would love if we can bring this feature in!

@its28604
Copy link
Author

Hi @mescanne ,

It's been a while since last comment, is there any update with this PR?

Best

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants