chore: transition the library to microgenerator - #62
Conversation
c252fba to
38e6c39
Compare
Not all paths are covered, not even in the generated code, thus the adjustment is necessary.
| # limitations under the License. | ||
| # | ||
|
|
||
| from .services.big_query_read import BigQueryReadClient |
There was a problem hiding this comment.
Let's have this be the manual wrapper.
There was a problem hiding this comment.
This is a generated file and bigquery_storage_v1/client.py imports from it. The latter assumes that the generated client is imported and wraps it, although it could also import directly from storage_v1.services.big_query_read, in principle.
Do you think it's worth maintaining additional synth replacement rules to prevent users from accessing the generated client through google.cloud.bigquery.storage_v1?
There was a problem hiding this comment.
Do you think it's worth maintaining additional synth replacement rules to prevent users from accessing the generated client through google.cloud.bigquery.storage_v1?
I'm pretty sure that's what I did for the bigquery_storage_v1beta1 package (that is, exclude the equivalent top-level __init__.py from getting copied over.
I'm still flip-flopping about what namespace we should use for the manual clients. It's nice to avoid breaking changes, but if we're breaking anyway, maybe we should be consistent. What do other manual libs do for namespace of manual clients?
There was a problem hiding this comment.
PubSub just exposes the generated clients through pubsub_v1.__init__.py import.
I don't know about other libs, but can check (I suspect they left these imports intact, though).
There was a problem hiding this comment.
Interesting! I find that it uses the google.pubsub_v1 for generated and google.cloud.pubsub_v1 for handwritten to be pretty confusing, but I guess so long as it's documented well, it'll be okay.
There was a problem hiding this comment.
I can at least mention this in the UPGRADING guide and just recommend to import from google.cloud.bigquery.storage (the samples use this path, too). This will give us more freedom to restructure the library internally in the future (in theory at least).
Update: FWIW, I found another library with hand-written layer that has already been migrated to microgenerator (AutoML) and it, too, exposes the generated client through google.cloud.automl_v1.
Rules conditionally matching versions other than v1 are not needed anymore.
|
Addressed all comments. Once the changes are finalized and confirmed, I will follow up with the UPGRADING guide. Update: Added the upgrade guide, as I think nothing significant will change anymore now that we discussed the import paths. |
|
Regarding the Sample tests, I just sent out internal CL 333505519 to add the missing Kokoro config. |
tswast
left a comment
There was a problem hiding this comment.
LGTM once those UPGRADING fixes are in.
Closes #61.
Remaining things:
v1beta1andv1beta2code (remove? YES!).