Feat: new cleaning rule to orphaned subscriptions - #39538
Conversation
…red orphaned subscriptions of taxirides topic
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
HansMarcus01
changed the title
|
Assigning reviewers: R: @shunping added as fallback since no labels match configuration Note: If you would like to opt out of this review, comment Available commands:
The PR bot will only process comments in the main thread (not review comments). |
|
can you upload the output from the dry run of your changes? so we can see expected results |
| if not self.prefixes or any(subscription_name.startswith(f"{self.project_path}/subscriptions/{prefix}") for prefix in self.prefixes): | ||
| # Check if the subscription has a topic associated with it | ||
| if subscription.detached: | ||
| # Safe orphan detection: |
There was a problem hiding this comment.
we need to be thoughtful about the behavior of this function.
The behavior before this change is:
- No prefixes are defined, so iterate through ALL subscriptions, and capture only detached ones. Is that in line with your understanding?
If we apply this change, we:
- No longer catch all detached subscriptions because prefixes are defined. Does that make sense?
- We are also adding a special case
if taxi...which should be covered by prefixes. This is turns the code into a totally strange thing.
So we have to change this function to something like this:
for s in list_subscriptions:
if subscription.detached = true:
d[s....]...
elif any(subscription_name.startswith(f"{self.project_path}/subscriptions/{prefix}") for prefix in self.prefixes):
d[s...]...
this covers our cases properly. Please go ahead and make that change.
|
|
||
| def test_active_resources(self): | ||
| """Test _active_resources method.""" | ||
| # Keep the existing standard prefix coverage and add the taxi-only cleanup prefix. |
There was a problem hiding this comment.
can you confirm that you ran this test? also, mark one sub with detached=True to make sure that case is covered.
…d active subscriptions under prefixes.
2 participants
This pull request introduces a new cleanup rule that identifies orphaned subscriptions for the topic related to taxirides.
Problem detected
taxirides-realtime_beam_-...was identified, connected to the public, continuous New York taxi topic.Solution
stale_cleaner.py:PubSubSubscriptionCleanerwas modified to evaluate and mark for deletion subscriptions with the prefixtaxirides-realtime_beam_that are older than 24 hours, even if they are not in thedetachedstate (since the NY taxi topic is permanent and public).test_stale_cleaner.py: Unit tests were added to validate this specific taxi-related criterion in isolation, ensuring that other subscriptions remain intact and safe.Output obtained
View test script and execution output
Command used: