Google-cloud-java: Package private toPb() methods

Created on 9 Sep 2019  路  7Comments  路  Source: googleapis/google-cloud-java

Is there any reason why the toPb() methods are package private? I have found myself in situations more than once where it would have been beneficial to have them as public.

(... and yes, I can work around it, I'm just curious what is the logic behind this.)

question

Most helpful comment

This might make sense, when it's very rare that someone wants to work with the actual Protobuf class, but what my issue usually looks like is that the class with the package private method just wraps a similar class from another library which might be actually in use in many projects/services/implementations. For example:
https://github.com/googleapis/google-cloud-java/blob/6666cdd62c0957afb5fb450e5fcf0fbe3e427225/google-cloud-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/JobInfo.java#L315-L334
The referenced class might be a Protobuf one - IDK, and haven't checked -, but TBH that is not the use-case it was needed for. What I needed was a com.google.api.services.bigquery.model.Job instance from a previously configured com.google.cloud.bigquery.Job, so I can further use it with code that expects classes from the legacy library, and not this one.

Still, okay, let's say I get it, the goal was to make it as noob-friendly as possible, but then at least make the workaround part of the project itself, and don't force devs who actually know what are they doing, to do extra work everytime they need to access a package private method.

All 7 comments

The users are not expected to work with Protobuf version of the model if there is a handwritten DSL available.

The handwritten models are designed to be user-friendly and focused to support relevant fields. Also, If at any point new fields are introduced on Protobuf which may be supported on other clients but not on java client. Then it might become hard to communicate the user about plans on start usage of those fields.

I think that is the general reasoning behind having client-specific models and keeping the Protobuf's package-private.

This might make sense, when it's very rare that someone wants to work with the actual Protobuf class, but what my issue usually looks like is that the class with the package private method just wraps a similar class from another library which might be actually in use in many projects/services/implementations. For example:
https://github.com/googleapis/google-cloud-java/blob/6666cdd62c0957afb5fb450e5fcf0fbe3e427225/google-cloud-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/JobInfo.java#L315-L334
The referenced class might be a Protobuf one - IDK, and haven't checked -, but TBH that is not the use-case it was needed for. What I needed was a com.google.api.services.bigquery.model.Job instance from a previously configured com.google.cloud.bigquery.Job, so I can further use it with code that expects classes from the legacy library, and not this one.

Still, okay, let's say I get it, the goal was to make it as noob-friendly as possible, but then at least make the workaround part of the project itself, and don't force devs who actually know what are they doing, to do extra work everytime they need to access a package private method.

@nbali Thanks for describing your use case. I understand you have a valid point, but I am sorry to inform you that we have no plans to provide any utility to allow access to these classes.

I am closing this issue as of now.

"[...] we have no plans to provide any utility to allow access to these classes [...]"

image

I mean this seems like a "just because" explanation. Just sayin'.

TBH I myself don't have the actual reasoning behind this decision, but I believe this was to keep the library flexible enough to accommodate future changes on proto files.

Also, Any public method introduced on this library needs to retain its definition & behavior until major version bump, and toPb() is expected to change with proto files but handwritten DSL doesn't need to.

@nbali I hope that answers your query.

Then @Beta (https://github.com/google/guava/blob/master/guava/src/com/google/common/annotations/Beta.java) should be added and that's all. Isn't this a perfect scenario for it?

Nowadays if you start a project with GCP you base your code on this library as it's much more convenient, but eventually you will need the toPb/fromPb method. It could be that you need a property/method this library doesn't support (yet), or it could be for interoperability with another library that uses the "Protobuf classes", whatever. It happens.

In that case I'm sure everybody would gladly use the @Beta public methods, instead of making them publicly available/calling them through reflection OR adding and maintaining the __very same code__ in their own codebase.

ps.: I even get that fromPb could be argued that it's risky - because it just silently skips any new property added to the proto classes, although @Beta should just cover that -, but toPb which uses every property from the current class to build the Protobuf instance shouldn't have that risk. Any other new property added to the Protobuf class isn't present at all so there is no "data loss", meanwhile I can't even remember a property being removed from an API object.

@nbali Sorry for the late response. I agree with your reasoning.

@kolea2, @ajaaym Would you please let us know your stand on these internal methods

Was this page helpful?
0 / 5 - 0 ratings