MartijnVisser commented on code in PR #28560:
URL: https://github.com/apache/flink/pull/28560#discussion_r4024528031
##########
flink-filesystems/flink-s3-fs-base/src/test/java/org/apache/flink/fs/s3/common/SeaweedFsTestContainer.java:
##########
@@ -71,21 +70,24 @@ public SeaweedFsTestContainer(String defaultBucketName) {
withEnv(AWS_ACCESS_KEY_ID, this.accessKey);
withEnv(AWS_SECRET_ACCESS_KEY, this.secretKey);
withCommand(
- "server", "-s3", "-s3.port=" + DEFAULT_PORT, "-dir=" +
DEFAULT_STORAGE_DIRECTORY);
+ "mini",
+ "-s3.port=" + DEFAULT_PORT,
+ "-dir=" + DEFAULT_STORAGE_DIRECTORY,
+ "-bucket=" + defaultBucketName);
+ // mini pre-creates the bucket and only reports readiness once every
component,
+ // including that bucket, is available.
setWaitStrategy(
- new HttpWaitStrategy()
- .forPort(DEFAULT_PORT)
- .forPath(HEALTH_ENDPOINT)
+ new LogMessageWaitStrategy()
+ .withRegEx("(?s).*All enabled components are running
and ready to use.*")
.withStartupTimeout(Duration.ofMinutes(2)));
- // Very rarely, a 503 status will be returned continuously while the
container is
- // starting up, slipping past the AmazonS3 client's default retry
strategy.
- withStartupAttempts(3);
}
@Override
protected void containerIsStarted(InspectContainerResponse containerInfo) {
super.containerIsStarted(containerInfo);
- createDefaultBucket();
+ Preconditions.checkState(
+ getClient().doesBucketExist(defaultBucketName),
Review Comment:
`doesBucketExist` is deprecated and returns true on any 403. SeaweedFS
answers a bad signature with 403, so this passes when the credentials break.
`doesBucketExistV2` throws there instead.
##########
flink-filesystems/flink-s3-fs-base/src/test/java/org/apache/flink/fs/s3/common/SeaweedFsTestContainer.java:
##########
@@ -71,21 +70,24 @@ public SeaweedFsTestContainer(String defaultBucketName) {
withEnv(AWS_ACCESS_KEY_ID, this.accessKey);
withEnv(AWS_SECRET_ACCESS_KEY, this.secretKey);
withCommand(
- "server", "-s3", "-s3.port=" + DEFAULT_PORT, "-dir=" +
DEFAULT_STORAGE_DIRECTORY);
+ "mini",
+ "-s3.port=" + DEFAULT_PORT,
+ "-dir=" + DEFAULT_STORAGE_DIRECTORY,
+ "-bucket=" + defaultBucketName);
+ // mini pre-creates the bucket and only reports readiness once every
component,
Review Comment:
This still claims readiness covers the bucket. The check you added below is
there because it does not, so the comment should go.
--
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]
