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:
Optional nit: 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.
--
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]
