gemini-code-assist[bot] commented on code in PR #35857:
URL: https://github.com/apache/beam/pull/35857#discussion_r2273171230
##########
sdks/python/apache_beam/transforms/enrichment_handlers/bigtable_it_test.py:
##########
@@ -167,7 +168,7 @@ def setUp(self):
instance = client.instance(self.instance_id)
self.table = instance.table(self.table_id)
create_rows(self.table)
- self.retries = 3
+ self.retries = 5
Review Comment:
The values `5` for retries and `2` for the sleep delay (in
`_start_container`) are magic numbers. Extracting them into named constants
(e.g., `_REDIS_STARTUP_RETRIES` and `_REDIS_STARTUP_RETRY_DELAY_S`) would make
the code's intent clearer and simplify future modifications.
##########
sdks/python/apache_beam/transforms/enrichment_handlers/bigquery_it_test.py:
##########
@@ -140,7 +140,7 @@ def setUpClass(cls):
def setUp(self) -> None:
self.condition_template = "id = {}"
- self.retries = 3
+ self.retries = 5
Review Comment:
The number of retries `5` and the sleep delay `2` (in `_start_container`)
are magic numbers. To improve readability and maintainability, consider
defining them as named constants at the module or class level, such as
`_REDIS_STARTUP_RETRIES` and `_REDIS_STARTUP_RETRY_DELAY_S`.
##########
sdks/python/apache_beam/transforms/enrichment_handlers/vertex_ai_feature_store_it_test.py:
##########
@@ -71,11 +72,11 @@ def setUp(self) -> None:
self.entity_type_name = "entity_id"
self.api_endpoint = "us-central1-aiplatform.googleapis.com"
self.feature_ids = ['title', 'genres']
- self.retries = 3
+ self.retries = 5
Review Comment:
The hardcoded values `5` for retries and `2` for the sleep delay (in
`_start_container`) should be defined as named constants. This practice
improves code clarity and makes it easier to adjust these parameters
consistently across tests.
##########
sdks/python/apache_beam/io/requestresponse_it_test.py:
##########
@@ -206,7 +207,7 @@ def __exit__(self, exc_type, exc_val, exc_tb):
@pytest.mark.uses_testcontainer
class TestRedisCache(unittest.TestCase):
def setUp(self) -> None:
- self.retries = 3
+ self.retries = 5
Review Comment:
Using a magic number `5` for retries can make the code harder to read and
maintain. It's better to define this as a named constant, for example
`_REDIS_STARTUP_RETRIES`, at the class or module level. The same applies to the
sleep delay of `2` seconds in `_start_container`.
--
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: github-unsubscr...@beam.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
