Feat: new cleaning rule to orphaned subscriptions - #39538
Merged
Conversation
…red orphaned subscriptions of taxirides topic
Contributor
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
HansMarcus01
changed the title
Contributor
|
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). |
Member
|
can you upload the output from the dry run of your changes? so we can see expected results |
pabloem
reviewed
Jul 29, 2026
…d active subscriptions under prefixes.
pabloem
requested changes
Jul 30, 2026
pabloem
requested changes
Aug 2, 2026
pabloem
left a comment
pabloem
left a comment
Member
There was a problem hiding this comment.
can you confirm that you ran the unittest and can you confirm that you have posted the latest output of the script? also fix comments in spanish.
| sub1.name = "projects/test-project/subscriptions/test-prefix-sub1" | ||
| sub1.topic = "projects/test-project/topics/some-topic" | ||
| def test_active_resources_active_subscriptions(self): | ||
| """Valida que las suscripciones activas con el prefijo de taxirides sean identificadas.""" |
| sub2 = mock.Mock() | ||
| sub2.name = "projects/test-project/subscriptions/test-prefix-sub2-detached" | ||
| sub2.topic = "_deleted-topic_" | ||
| # Suscripción activa con el prefijo correcto de taxis |
| sub3 = mock.Mock() | ||
| sub3.name = "projects/test-project/subscriptions/other-prefix-sub3" | ||
| sub3.topic = "projects/test-project/topics/another-topic" | ||
| # Suscripción activa con un prefijo distinto |
| self.assertIn("projects/test-project/subscriptions/test-prefix-sub2-detached", active) | ||
| self.assertNotIn("projects/test-project/subscriptions/other-prefix-sub3", active) | ||
| self.assertEqual(len(active), 2) | ||
| # Verificamos que solo capture la suscripción de taxi, descartando la otra |
| self.assertEqual(len(active), 1) | ||
|
|
||
| def test_active_resources_detached_subscriptions(self): | ||
| """Valida que las suscripciones desconectadas se mantengan en la lista de activos a limpiar, |
| independientemente de los prefijos específicos de taxis.""" | ||
| self.cleaner.prefixes = ["test-prefix"] | ||
|
|
||
| # Suscripción desconectada (debería incluirse en la recolección) |
| sub_detached.topic = "_deleted-topic_" | ||
| sub_detached.detached = True | ||
|
|
||
| # Suscripción conectada normal (debería ignorarse) |
| with SilencePrint(): | ||
| active = self.cleaner._active_resources() | ||
|
|
||
| # Verificamos que solo se registre la suscripción huérfana |
Member
|
lgtm |
rwiggles
pushed a commit
to rwiggles/beam-rwiggles
that referenced
this pull request
Aug 5, 2026
* Feat: A new prefix was added to the cleaner to clean up newly discovered orphaned subscriptions of taxirides topic * Fix: Modifications were made to manage the cleanup of disconnected and active subscriptions under prefixes. * Fix: Deleting the hardcode about taxi prefix * Fix: Delete comment in spanish * Fix: resolving the issue where active subscriptions are not deleted
rwiggles
pushed a commit
to rwiggles/beam-rwiggles
that referenced
this pull request
Aug 6, 2026
* Feat: A new prefix was added to the cleaner to clean up newly discovered orphaned subscriptions of taxirides topic * Fix: Modifications were made to manage the cleanup of disconnected and active subscriptions under prefixes. * Fix: Deleting the hardcode about taxi prefix * Fix: Delete comment in spanish * Fix: resolving the issue where active subscriptions are not deleted
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.The document describing the justification for cleaning this resource can be found at the following link: https://gist.github.com/HansMarcus01/101c53d0e9fa6179a5f631b59324df96
Test results
The tests were executed successfully and without errors.
Output obtained on August 3, 2026
View test script and execution output
Command used: