feat(datastore): add support for request tags#13732
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces support for passing and merging RequestOptions (such as request tags) in Datastore queries and aggregation queries, allowing both instance-level and request-level tags to be propagated. The feedback highlights a critical issue where instance-level tags are ignored when query-level RequestOptions are not explicitly provided in QueryResultsImpl and AggregationQueryRequestProtoPreparer. Additionally, the reviewer suggests simplifying the varargs withRequestTags overload in Datastore by delegating to the List version, and adding a test case to verify that instance-level tags are correctly populated and sent with queries.
f124116 to
aab1fd3
Compare
|
ci / lint is failing. Could you please take a look? |
Fixed |
d61b477 to
75e872b
Compare
75e872b to
ee402b8
Compare
ee402b8 to
88cc471
Compare
|
Thanks! I think the changes are looking good. I've added another round of comments and smaller changes for readability. One question that probably depends on plans regarding stability of request tags: When do we feel comfortable marking it from @BetaApi to GA? I'm thinking that the methods in the Datastore interface that do not have DatastoreExecutionOptions probably should be marked with Perhaps it may be better to mark the requestTag getters/ settings with @BetaApi instead? Since 2/3 of DatastoreExecutionOptions (Explain and Read Options are stable, just the request tags is new) |
c2e4695 to
ea088e1
Compare
… in DatastoreImpl
ea088e1 to
c2d9004
Compare
We are planning to do a public preview of supporting request tags , the backend changes are already enabled. Should I create a separate bug for doing what you suggested? |
This PR introduced sending request tags for different datastore requests. The tags can be passed using 2 mechanisms: