fix: accept uniqueWriterIdentity in setMetadata#1034
Conversation
google-cla
Bot
added
the
cla: yes
product-auto-label
Bot
added
the
api: logging
freelerobot
added
priority: p1
| /** | ||
| * Set the sink's metadata. | ||
| * | ||
| * Note: If the sink was created with uniqueWriterIdentity = true, then the |
There was a problem hiding this comment.
To me, this reads like the user will have to provide uniqueWriterIdentity = true, but it looks like the code does it for them. Should this say something like "Note: If the sink was created with uniqueWriterIdentity = true, we will automatically apply this option during the setMetadata operation."?
There was a problem hiding this comment.
We dont automatically apply this in the setMetadata operation yet. Hence this issue, but I need to double check with logging-eng whether my proposed detection heuristic is reliable. For the scope of this PR, I aim to unblock the user but letting them set the boolean manually.
Will update the comment to be clearer :)
Codecov Report
@@ Coverage Diff @@
## master #1034 +/- ##
=======================================
Coverage 97.89% 97.90%
=======================================
Files 18 18
Lines 13224 13239 +15
Branches 410 412 +2
=======================================
+ Hits 12946 12961 +15
Misses 275 275
Partials 3 3
Continue to review full report at Codecov.
|
| assert.strictEqual(apiResponse.filter, FILTER); | ||
| // Sink must be deleted within this test before any logs are generated | ||
| // to avoid topic_permission_denied emails. | ||
| await sink.delete(); |
There was a problem hiding this comment.
Can you have a finally block to always delete the sink at the end of the test?
There was a problem hiding this comment.
I have an after block to delete sinks for the other tests. But these uniqueId sinks require a reference to the sink names created within the scope of the tests.
freelerobot
added
the
automerge
gcf-merge-on-green
Bot
removed
the
automerge
3 participants
Final Spec (internal googler link). Fixes: b/183619158
Changes: