feat(bigquery/storage/managedwriter): improve method parity in managedwriter #5007
Conversation
google-cla
bot
added
the
cla: yes
shollyman
changed the title
shollyman
changed the title
product-auto-label
bot
added
the
api: bigquery
| // CreateWriteStream. It is a stream that can be used simultaneously by any | ||
| // number of clients. Data written to this stream is considered committed as | ||
| // soon as an acknowledgement is received. | ||
| func (c *Client) CreateWriteStream(ctx context.Context, req *storagepb.CreateWriteStreamRequest, opts ...gax.CallOption) (*storagepb.WriteStream, error) { |
There was a problem hiding this comment.
Do you think we should be creating wrapper types for req objs? One think I generally like our manual layers is that a user does not have to import two namespaces to interact with our clients, but when we other package types in the parameters they do. WDYT?
There was a problem hiding this comment.
That's the bit I'm struggling with. This service in general does lots of subtle things with protobuf, so the expectation is that users will be more aware of those kinds of interactions.
The other strategy would be to keep the wrapped batch commit method, and extend ManagedWriter further to allow for things like mutable schemas, and mechanisms for returning existing table schemas.
There was a problem hiding this comment.
I'm not super enamored of just copying the request protos themselves, but I suppose that's another option.
There was a problem hiding this comment.
👍🏻 Seems fine for now. Might want to take another look at the whole surface before it is stabilized.
There was a problem hiding this comment.
Yeah, that's definitely warranted.
shollyman
added
the
automerge
gcf-merge-on-green
bot
removed
the
automerge
This PR exposes the raw methods for creating and committing streams to the wrapped managedwriter client.
It allows users to interact with all the methods of the underlying API using the managedwriter client (which itself wraps the raw v1 client). The disadvantage is that it couples managedwriter directly to v1, as it accepts requests in the v1 namespace. The existing append interactions all use abstractions local to the managedwriter.
PR also gets rid of the utility method for batch committing write streams; there's not a great deal of utility saved here vs the underlying method.
Towards: #4366