- Notifications
You must be signed in to change notification settings - Fork 135
samples: add samples for tagging feature #1042
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| Here is the summary of changes. You are about to add 2 region tags.
This comment is generated by snippet-bot.
|
Codecov Report
@@ Coverage Diff @@ ## master #1042 +/- ## ============================================ + Coverage 85.18% 85.24% +0.05% - Complexity 2640 2658 +18 ============================================ Files 155 155 Lines 14413 14453 +40 Branches 1348 1362 +14 ============================================ + Hits 12278 12320 +42 + Misses 1567 1565 -2 Partials 568 568
Continue to review full report at Codecov.
|
thiagotnunes left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that the first sample spanner_set_transaction_and_request_tags does not entirely match the one we have in the client spec.
Is that true, or did we decide to change directions @larkee ?
samples/snippets/src/main/java/com/example/spanner/TransactionWithTagSample.java Outdated Show resolved Hide resolved
samples/snippets/src/main/java/com/example/spanner/TransactionWithTagSample.java Outdated Show resolved Hide resolved
samples/snippets/src/main/java/com/example/spanner/TransactionWithTagSample.java Show resolved Hide resolved
samples/snippets/src/main/java/com/example/spanner/TransactionWithTagSample.java Outdated Show resolved Hide resolved
samples/snippets/src/main/java/com/example/spanner/TransactionWithTagSample.java Outdated Show resolved Hide resolved
| // Execute query on Query statistics | ||
| // see: https://cloud.google.com/spanner/docs/introspection/query-statistics, for more details. | ||
| String sql = | ||
| "SELECT t.REQUEST_TAG, t.AVG_LATENCY_SECONDS, t.AVG_CPU_SECONDS " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I can tell, REQUEST_TAG is not yet available in (public) Cloud Spanner databases. This should therefore not be merged until the column is available.
Furthermore: The t. prefix here does not work. There's no alias t in the query below.
Updated the sample wrt the client spec. PTAL |
| @mayurkale22 could you add some tests to this sample? We would like to make sure everything is working correctly before merging it. |
Sure, I will add test soon. I will wait until the new column becomes visible for tests. |
Adds a sample for assigning and querying tags.
Follow-up to #576
Note: We will add the test when the feature is fully available.