doc(pubsub): add update topic schema and create topic with schema revisions to samples #11872
Conversation
|
Here is the summary of changes. You are about to add 2 region tags.
This comment is generated by snippet-bot.
|
1fe6382 to
410f4f1
Compare
| create_request.mutable_schema()->set_definition(initial_definition); | ||
| auto schema = client.CreateSchema(create_request); | ||
| if (!schema) throw std::move(schema).status(); | ||
| std::string const first_revision_id = schema.value().revision_id(); |
There was a problem hiding this comment.
We spell these schema->revision_id(). Also, making this a const prevents move-from. Does not matter in this test, but generally it would be more efficient to write:
... ..
auto first_revision_id = schema->value();
...
return {std::move(first_revision_id), std::move(last_revision_id)}If you want to save even more copying you would write:
auto first_revision_id = std::move(*schema->mutable_revision_id());
..
...
return {std::move(first_revision_id), std::move(*schema->mutable_revision_id())};There was a problem hiding this comment.
Ack. For StatusOr's, use indirection instead of .value().
Changed so that we are using std::move in the return. I think the second version is overkill for the samples.
| if (!schema) throw std::move(schema).status(); | ||
| return first_revision_id; | ||
| std::string const last_revision_id = schema.value().revision_id(); |
| // Commit two new schema revision and return the first revision id. | ||
| std::string CommitSchemaRevisionsForRollbackSchemaTesting( | ||
| google::cloud::pubsub::SchemaServiceClient& schema_admin, | ||
| // Commit a schema with a revision and return the first and last revision id. |
There was a problem hiding this comment.
s/revision id/revision ids/ ?
| if (topic.status().code() == google::cloud::StatusCode::kAlreadyExists) { | ||
| std::cout << "The topic already exists\n"; | ||
| return; | ||
| } else if (topic.status().code() == |
There was a problem hiding this comment.
We typically write
| if (topic.status().code() == google::cloud::StatusCode::kAlreadyExists) { | |
| std::cout << "The topic already exists\n"; | |
| return; | |
| } else if (topic.status().code() == | |
| return; | |
| } | |
| if (topic.status().code() == google::cloud::StatusCode::kInvalidArgument) { |
There was a problem hiding this comment.
Ack. Removed since the code is thrown says the same thing.
| .set_first_revision_id(first_revision_id) | ||
| .set_last_revision_id(last_revision_id)); | ||
|
|
||
| if (topic.status().code() == google::cloud::StatusCode::kInvalidArgument) { |
There was a problem hiding this comment.
There are a multitude of reasons why we may get this error, just skip this if() I think.
| std::cout << "The topic already exists\n"; | ||
| return; | ||
| } else if (topic.status().code() == | ||
| google::cloud::StatusCode::kInvalidArgument) { |
There was a problem hiding this comment.
I am not sure the message for kInvalidArgument is accurate, or more informative than the message in topic.status()?
There was a problem hiding this comment.
The messages are the same if the revision ids are incorrect, but it could be something else that triggers the error.
Instead of trying to catch the error, changed to rely on throwing the error at the end
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #11872 +/- ##
==========================================
- Coverage 93.67% 93.66% -0.01%
==========================================
Files 1838 1838
Lines 166116 166195 +79
==========================================
+ Hits 155601 155670 +69
- Misses 10515 10525 +10
☔ View full report in Codecov by Sentry. |
Fixes part of Issue #11720
This change is