pabloem commented on code in PR #39538:
URL: https://github.com/apache/beam/pull/39538#discussion_r3698364401
##########
.test-infra/tools/test_stale_cleaner.py:
##########
@@ -431,30 +431,58 @@ def test_init(self):
self.assertEqual(self.cleaner.time_threshold, self.time_threshold)
self.assertIsInstance(self.cleaner.clock, FakeClock)
- def test_active_resources(self):
- """Test _active_resources method."""
- # Mock subscriptions
- sub1 = mock.Mock()
- 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."""
Review Comment:
comments should be in english.
##########
.test-infra/tools/test_stale_cleaner.py:
##########
@@ -431,30 +431,58 @@ def test_init(self):
self.assertEqual(self.cleaner.time_threshold, self.time_threshold)
self.assertIsInstance(self.cleaner.clock, FakeClock)
- def test_active_resources(self):
- """Test _active_resources method."""
- # Mock subscriptions
- sub1 = mock.Mock()
- 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."""
+ self.cleaner.prefixes = ["taxirides-realtime_beam_"]
- 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
Review Comment:
english
##########
.test-infra/tools/test_stale_cleaner.py:
##########
@@ -431,30 +431,58 @@ def test_init(self):
self.assertEqual(self.cleaner.time_threshold, self.time_threshold)
self.assertIsInstance(self.cleaner.clock, FakeClock)
- def test_active_resources(self):
- """Test _active_resources method."""
- # Mock subscriptions
- sub1 = mock.Mock()
- 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."""
+ self.cleaner.prefixes = ["taxirides-realtime_beam_"]
- 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
+ sub_taxi_active = mock.Mock()
+ sub_taxi_active.name =
f"projects/{self.project_id}/subscriptions/taxirides-realtime_beam_-12345"
+ sub_taxi_active.topic =
"projects/pubsub-public-data/topics/taxirides-realtime"
+ sub_taxi_active.detached = False
- 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
Review Comment:
english
##########
.test-infra/tools/test_stale_cleaner.py:
##########
@@ -431,30 +431,58 @@ def test_init(self):
self.assertEqual(self.cleaner.time_threshold, self.time_threshold)
self.assertIsInstance(self.cleaner.clock, FakeClock)
- def test_active_resources(self):
- """Test _active_resources method."""
- # Mock subscriptions
- sub1 = mock.Mock()
- 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."""
+ self.cleaner.prefixes = ["taxirides-realtime_beam_"]
- 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
+ sub_taxi_active = mock.Mock()
+ sub_taxi_active.name =
f"projects/{self.project_id}/subscriptions/taxirides-realtime_beam_-12345"
+ sub_taxi_active.topic =
"projects/pubsub-public-data/topics/taxirides-realtime"
+ sub_taxi_active.detached = False
- 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
+ sub_other_active = mock.Mock()
+ sub_other_active.name =
f"projects/{self.project_id}/subscriptions/other-prefix-sub"
+ sub_other_active.topic =
f"projects/{self.project_id}/topics/another-topic"
+ sub_other_active.detached = False
- self.mock_subscriber_client.list_subscriptions.return_value = [sub1,
sub2, sub3]
+ self.mock_subscriber_client.list_subscriptions.return_value =
[sub_taxi_active, sub_other_active]
with SilencePrint():
active = self.cleaner._active_resources()
- self.assertIn("projects/test-project/subscriptions/test-prefix-sub1",
active)
-
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
Review Comment:
ditto
##########
.test-infra/tools/test_stale_cleaner.py:
##########
@@ -431,30 +431,58 @@ def test_init(self):
self.assertEqual(self.cleaner.time_threshold, self.time_threshold)
self.assertIsInstance(self.cleaner.clock, FakeClock)
- def test_active_resources(self):
- """Test _active_resources method."""
- # Mock subscriptions
- sub1 = mock.Mock()
- 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."""
+ self.cleaner.prefixes = ["taxirides-realtime_beam_"]
- 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
+ sub_taxi_active = mock.Mock()
+ sub_taxi_active.name =
f"projects/{self.project_id}/subscriptions/taxirides-realtime_beam_-12345"
+ sub_taxi_active.topic =
"projects/pubsub-public-data/topics/taxirides-realtime"
+ sub_taxi_active.detached = False
- 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
+ sub_other_active = mock.Mock()
+ sub_other_active.name =
f"projects/{self.project_id}/subscriptions/other-prefix-sub"
+ sub_other_active.topic =
f"projects/{self.project_id}/topics/another-topic"
+ sub_other_active.detached = False
- self.mock_subscriber_client.list_subscriptions.return_value = [sub1,
sub2, sub3]
+ self.mock_subscriber_client.list_subscriptions.return_value =
[sub_taxi_active, sub_other_active]
with SilencePrint():
active = self.cleaner._active_resources()
- self.assertIn("projects/test-project/subscriptions/test-prefix-sub1",
active)
-
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.assertIn(sub_taxi_active.name, active)
+ self.assertNotIn(sub_other_active.name, active)
+ 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,
Review Comment:
english
##########
.test-infra/tools/test_stale_cleaner.py:
##########
@@ -431,30 +431,58 @@ def test_init(self):
self.assertEqual(self.cleaner.time_threshold, self.time_threshold)
self.assertIsInstance(self.cleaner.clock, FakeClock)
- def test_active_resources(self):
- """Test _active_resources method."""
- # Mock subscriptions
- sub1 = mock.Mock()
- 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."""
+ self.cleaner.prefixes = ["taxirides-realtime_beam_"]
- 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
+ sub_taxi_active = mock.Mock()
+ sub_taxi_active.name =
f"projects/{self.project_id}/subscriptions/taxirides-realtime_beam_-12345"
+ sub_taxi_active.topic =
"projects/pubsub-public-data/topics/taxirides-realtime"
+ sub_taxi_active.detached = False
- 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
+ sub_other_active = mock.Mock()
+ sub_other_active.name =
f"projects/{self.project_id}/subscriptions/other-prefix-sub"
+ sub_other_active.topic =
f"projects/{self.project_id}/topics/another-topic"
+ sub_other_active.detached = False
- self.mock_subscriber_client.list_subscriptions.return_value = [sub1,
sub2, sub3]
+ self.mock_subscriber_client.list_subscriptions.return_value =
[sub_taxi_active, sub_other_active]
with SilencePrint():
active = self.cleaner._active_resources()
- self.assertIn("projects/test-project/subscriptions/test-prefix-sub1",
active)
-
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.assertIn(sub_taxi_active.name, active)
+ self.assertNotIn(sub_other_active.name, active)
+ 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 = mock.Mock()
+ sub_detached.name =
f"projects/{self.project_id}/subscriptions/test-prefix-detached"
+ sub_detached.topic = "_deleted-topic_"
+ sub_detached.detached = True
+
+ # Suscripción conectada normal (debería ignorarse)
Review Comment:
english
##########
.test-infra/tools/test_stale_cleaner.py:
##########
@@ -431,30 +431,58 @@ def test_init(self):
self.assertEqual(self.cleaner.time_threshold, self.time_threshold)
self.assertIsInstance(self.cleaner.clock, FakeClock)
- def test_active_resources(self):
- """Test _active_resources method."""
- # Mock subscriptions
- sub1 = mock.Mock()
- 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."""
+ self.cleaner.prefixes = ["taxirides-realtime_beam_"]
- 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
+ sub_taxi_active = mock.Mock()
+ sub_taxi_active.name =
f"projects/{self.project_id}/subscriptions/taxirides-realtime_beam_-12345"
+ sub_taxi_active.topic =
"projects/pubsub-public-data/topics/taxirides-realtime"
+ sub_taxi_active.detached = False
- 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
+ sub_other_active = mock.Mock()
+ sub_other_active.name =
f"projects/{self.project_id}/subscriptions/other-prefix-sub"
+ sub_other_active.topic =
f"projects/{self.project_id}/topics/another-topic"
+ sub_other_active.detached = False
- self.mock_subscriber_client.list_subscriptions.return_value = [sub1,
sub2, sub3]
+ self.mock_subscriber_client.list_subscriptions.return_value =
[sub_taxi_active, sub_other_active]
with SilencePrint():
active = self.cleaner._active_resources()
- self.assertIn("projects/test-project/subscriptions/test-prefix-sub1",
active)
-
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.assertIn(sub_taxi_active.name, active)
+ self.assertNotIn(sub_other_active.name, active)
+ 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 = mock.Mock()
+ sub_detached.name =
f"projects/{self.project_id}/subscriptions/test-prefix-detached"
+ sub_detached.topic = "_deleted-topic_"
+ sub_detached.detached = True
+
+ # Suscripción conectada normal (debería ignorarse)
+ sub_attached = mock.Mock()
+ sub_attached.name =
f"projects/{self.project_id}/subscriptions/test-prefix-attached"
+ sub_attached.topic =
f"projects/{self.project_id}/topics/some-topic"
+ sub_attached.detached = False
+
+ self.mock_subscriber_client.list_subscriptions.return_value =
[sub_detached, sub_attached]
+
+ with SilencePrint():
+ active = self.cleaner._active_resources()
+
+ # Verificamos que solo se registre la suscripción huérfana
Review Comment:
english
##########
.test-infra/tools/test_stale_cleaner.py:
##########
@@ -431,30 +431,58 @@ def test_init(self):
self.assertEqual(self.cleaner.time_threshold, self.time_threshold)
self.assertIsInstance(self.cleaner.clock, FakeClock)
- def test_active_resources(self):
- """Test _active_resources method."""
- # Mock subscriptions
- sub1 = mock.Mock()
- 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."""
+ self.cleaner.prefixes = ["taxirides-realtime_beam_"]
- 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
+ sub_taxi_active = mock.Mock()
+ sub_taxi_active.name =
f"projects/{self.project_id}/subscriptions/taxirides-realtime_beam_-12345"
+ sub_taxi_active.topic =
"projects/pubsub-public-data/topics/taxirides-realtime"
+ sub_taxi_active.detached = False
- 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
+ sub_other_active = mock.Mock()
+ sub_other_active.name =
f"projects/{self.project_id}/subscriptions/other-prefix-sub"
+ sub_other_active.topic =
f"projects/{self.project_id}/topics/another-topic"
+ sub_other_active.detached = False
- self.mock_subscriber_client.list_subscriptions.return_value = [sub1,
sub2, sub3]
+ self.mock_subscriber_client.list_subscriptions.return_value =
[sub_taxi_active, sub_other_active]
with SilencePrint():
active = self.cleaner._active_resources()
- self.assertIn("projects/test-project/subscriptions/test-prefix-sub1",
active)
-
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.assertIn(sub_taxi_active.name, active)
+ self.assertNotIn(sub_other_active.name, active)
+ 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)
Review Comment:
english
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
