damccorm commented on code in PR #41129:
URL: https://github.com/apache/beam/pull/41129#discussion_r4517182817
##########
sdks/python/apache_beam/transforms/enrichment_handlers/cloudsql_it_test.py:
##########
@@ -148,9 +149,28 @@ def start_sql_db_container(
info.address)
break
except Exception as e:
- stdout_logs, stderr_logs = sql_db_container.get_logs()
- stdout_logs = stdout_logs.decode("utf-8")
- stderr_logs = stderr_logs.decode("utf-8")
+ stdout_logs = stderr_logs = ""
+ if getattr(sql_db_container, "_container", None) is not None:
+ try:
+ raw_out, raw_err = sql_db_container.get_logs()
+ stdout_logs = raw_out.decode("utf-8")
+ stderr_logs = raw_err.decode("utf-8")
+ except Exception:
+ pass
+ try:
+ sql_db_container.stop()
+ except Exception:
+ pass
+ if getattr(Reaper, "_instance", None) is None:
+ try:
+ docker_client = sql_db_container.get_docker_client()
+ session_id = docker_client.client.api.headers.get("x-tc-sid")
+ if session_id:
+ ryuk_container = docker_client.client.containers.get(
+ f"testcontainers-ryuk-{session_id}")
+ ryuk_container.remove(force=True)
+ except Exception:
+ pass
Review Comment:
This ~25-line failure recovery block (best-effort `get_logs()` + `stop()` +
orphaned `testcontainers-ryuk-{session_id}` cleanup) is duplicated verbatim in
`sdks/python/apache_beam/ml/rag/test_utils.py` (and other testcontainer retry
loops like `BaseTestSQLEnrichment._start_cache_container` below or
`xlang_jdbcio_it_test.py` don't include the Ryuk cleanup). Consider extracting
a small shared helper (e.g., in `sdks/python/apache_beam/testing/test_utils.py`
alongside `require_linux_docker`) so the private `testcontainers` attribute
accesses (`_container`, `Reaper._instance`, `x-tc-sid`) live in one place.
##########
sdks/python/apache_beam/transforms/enrichment_handlers/cloudsql_it_test.py:
##########
@@ -148,9 +149,28 @@ def start_sql_db_container(
info.address)
break
except Exception as e:
- stdout_logs, stderr_logs = sql_db_container.get_logs()
- stdout_logs = stdout_logs.decode("utf-8")
- stderr_logs = stderr_logs.decode("utf-8")
+ stdout_logs = stderr_logs = ""
+ if getattr(sql_db_container, "_container", None) is not None:
+ try:
+ raw_out, raw_err = sql_db_container.get_logs()
+ stdout_logs = raw_out.decode("utf-8")
+ stderr_logs = raw_err.decode("utf-8")
+ except Exception:
+ pass
+ try:
+ sql_db_container.stop()
+ except Exception:
+ pass
+ if getattr(Reaper, "_instance", None) is None:
+ try:
+ docker_client = sql_db_container.get_docker_client()
+ session_id = docker_client.client.api.headers.get("x-tc-sid")
+ if session_id:
+ ryuk_container = docker_client.client.containers.get(
+ f"testcontainers-ryuk-{session_id}")
+ ryuk_container.remove(force=True)
+ except Exception:
+ pass
Review Comment:
This ~25-line failure recovery block (best-effort `get_logs()` + `stop()` +
orphaned `testcontainers-ryuk-{session_id}` cleanup) is duplicated verbatim in
`sdks/python/apache_beam/ml/rag/test_utils.py` (and other testcontainer retry
loops like `BaseTestSQLEnrichment._start_cache_container` below or
`xlang_jdbcio_it_test.py` don't include the Ryuk cleanup). Extract a small
shared helper (e.g., in `sdks/python/apache_beam/testing/test_utils.py`
alongside `require_linux_docker`) so the private `testcontainers` attribute
accesses (`_container`, `Reaper._instance`, `x-tc-sid`) live in one place.
--
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]
