Copilot commented on code in PR #13029:
URL: https://github.com/apache/seatunnel/pull/13029#discussion_r4401146196
##########
seatunnel-e2e/seatunnel-connector-v2-e2e/connector-amazonsqs-e2e/src/test/java/org/apache/seatunnel/e2e/connector/amazonsqs/AmazonsqsIT.java:
##########
@@ -68,7 +71,21 @@ public class AmazonsqsIT extends TestSuiteBase implements
TestResource {
public void startUp() throws Exception {
// start a localstack docker container
localstack =
- new LocalStackContainer(LOCALSTACK_DOCKER_IMAGE_VERSION)
+ new LocalStackContainer(LOCALSTACK_DOCKER_IMAGE_VERSION) {
+ @Override
+ protected void
containerIsStarting(InspectContainerResponse containerInfo) {
+ super.containerIsStarting(containerInfo);
+ // Script existence alone does not mean Docker has
finished copying it.
+ copyFileToContainer(Transferable.of(new byte[0]),
LOCALSTACK_STARTER_READY);
+ }
+ }.withCreateContainerCmdModifier(
+ cmd ->
+ cmd.withEntrypoint(
+ "sh",
+ "-c",
+ "while [ ! -f "
+ +
LOCALSTACK_STARTER_READY
+ + " ]; do sleep 0.1;
done; exec /testcontainers_start.sh"))
Review Comment:
Using `sleep 0.1` relies on fractional sleep support in the container's
`sh`/`sleep` implementation, which is not guaranteed across all minimal
images/environments. Using an integer sleep (e.g., `sleep 1`) is more portable;
if faster polling is needed, consider an approach that doesn't depend on
fractional sleep availability.
##########
seatunnel-e2e/seatunnel-connector-v2-e2e/connector-amazonsqs-e2e/src/test/java/org/apache/seatunnel/e2e/connector/amazonsqs/AmazonsqsIT.java:
##########
@@ -68,7 +71,21 @@ public class AmazonsqsIT extends TestSuiteBase implements
TestResource {
public void startUp() throws Exception {
// start a localstack docker container
localstack =
- new LocalStackContainer(LOCALSTACK_DOCKER_IMAGE_VERSION)
+ new LocalStackContainer(LOCALSTACK_DOCKER_IMAGE_VERSION) {
+ @Override
+ protected void
containerIsStarting(InspectContainerResponse containerInfo) {
+ super.containerIsStarting(containerInfo);
+ // Script existence alone does not mean Docker has
finished copying it.
+ copyFileToContainer(Transferable.of(new byte[0]),
LOCALSTACK_STARTER_READY);
+ }
+ }.withCreateContainerCmdModifier(
+ cmd ->
+ cmd.withEntrypoint(
+ "sh",
+ "-c",
+ "while [ ! -f "
+ +
LOCALSTACK_STARTER_READY
+ + " ]; do sleep 0.1;
done; exec /testcontainers_start.sh"))
Review Comment:
The shell command is assembled via multi-line string concatenation, which is
relatively hard to read and easy to break when edited. Consider extracting it
into a named constant (or building it with a single formatted string) to make
the intent and exact command easier to verify.
##########
seatunnel-e2e/seatunnel-connector-v2-e2e/connector-amazonsqs-e2e/src/test/java/org/apache/seatunnel/e2e/connector/amazonsqs/AmazonsqsIT.java:
##########
@@ -68,7 +71,21 @@ public class AmazonsqsIT extends TestSuiteBase implements
TestResource {
public void startUp() throws Exception {
// start a localstack docker container
localstack =
- new LocalStackContainer(LOCALSTACK_DOCKER_IMAGE_VERSION)
+ new LocalStackContainer(LOCALSTACK_DOCKER_IMAGE_VERSION) {
+ @Override
+ protected void
containerIsStarting(InspectContainerResponse containerInfo) {
+ super.containerIsStarting(containerInfo);
+ // Script existence alone does not mean Docker has
finished copying it.
+ copyFileToContainer(Transferable.of(new byte[0]),
LOCALSTACK_STARTER_READY);
+ }
+ }.withCreateContainerCmdModifier(
+ cmd ->
+ cmd.withEntrypoint(
+ "sh",
+ "-c",
+ "while [ ! -f "
+ +
LOCALSTACK_STARTER_READY
+ + " ]; do sleep 0.1;
done; exec /testcontainers_start.sh"))
Review Comment:
The entrypoint loop has no internal timeout/fail-fast behavior. If the
marker is never created (e.g., an unexpected early failure during container
startup/copy), the container will spin until an external timeout kills it,
which is harder to diagnose from logs. Consider adding a bounded wait (with a
clear error message and non-zero exit) to make failures deterministic and
easier to troubleshoot.
--
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]
