You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2020/12/01 12:48:41 UTC
[GitHub] [flink] dawidwys opened a new pull request #14275: [FLINK-20432] Add timeouts to SQLClientSchemaRegistryITCase
dawidwys opened a new pull request #14275:
URL: https://github.com/apache/flink/pull/14275
## What is the purpose of the change
The `SQLClientSchemaRegistryITCase` hanged on azure. I added timeouts to the test in order to fail more gracefully in case of taking excessive amount of time. I hope this will give us a bit more contextual info if it fails again, as the logs from the failure we had does not give us any hint.
## Brief change log
* I split creation of the Flink container image to two separate stages to speed it up a bit. Copying the flink-dist into the docker image takes some time, but at the same time it is immutable across runs.
* Added timeouts to test methods, as well as a global for the entire `SQLClientSchemaRegistryITCase` class.
## Verifying this change
This change improves a test.
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): (yes / **no**)
- The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (yes / **no**)
- The serializers: (yes / **no** / don't know)
- The runtime per-record code paths (performance sensitive): (yes / **no** / don't know)
- Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn/Mesos, ZooKeeper: (yes / **no** / don't know)
- The S3 file system connector: (yes / **no** / don't know)
## Documentation
- Does this pull request introduce a new feature? (yes / **no**)
- If yes, how is the feature documented? (**not applicable** / docs / JavaDocs / not documented)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] kl0u commented on a change in pull request #14275: [FLINK-20432] Add timeouts to SQLClientSchemaRegistryITCase
Posted by GitBox <gi...@apache.org>.
kl0u commented on a change in pull request #14275:
URL: https://github.com/apache/flink/pull/14275#discussion_r535182027
##########
File path: flink-end-to-end-tests/flink-end-to-end-tests-common/src/main/java/org/apache/flink/tests/util/flink/FlinkContainer.java
##########
@@ -256,31 +258,83 @@ public FlinkContainer build() throws IOException {
.mapToObj(i -> "localhost")
.collect(Collectors.toList()));
- ImageFromDockerfile image = new ImageFromDockerfile("flink-dist", true)
- .withDockerfileFromBuilder(
- builder -> {
- builder.from("openjdk:" + getJavaVersionSuffix())
- .copy(flinkDistName, "flink")
- .copy(flinkDistName + "/conf/workers", "workers")
- .cmd(FLINK_BIN + "/start-cluster.sh && tail -f /dev/null")
- .build();
- }
- )
- .withFileFromPath("workers", workersFile)
- .withFileFromPath(flinkDistName, flinkDist);
+ // Building the docker image is split into two stages:
+ // 1. build a base image with an immutable flink-dist
+ // 2. based on the base image add any mutable files such as e.g. workers files
+ //
+ // This lets us save some time for archiving and copying big, immutable files
+ // between tests runs.
+ String baseImage = buildBaseImage(flinkDist, flinkDistName);
+ ImageFromDockerfile configuredImage = buildConfiguredImage(
+ flinkDistName,
+ workersFile,
+ baseImage);
Optional<Path> logBackupDirectory = DISTRIBUTION_LOG_BACKUP_DIRECTORY.get();
if (!logBackupDirectory.isPresent()) {
LOG.warn(
"Property {} not set, logs will not be backed up in case of test failures.",
DISTRIBUTION_LOG_BACKUP_DIRECTORY.getPropertyName());
}
- return new FlinkContainer(image, numTaskManagers, logBackupDirectory.orElse(null));
+ return new FlinkContainer(
+ configuredImage,
+ numTaskManagers,
+ logBackupDirectory.orElse(null));
+ } catch (Exception e) {
+ throw new RuntimeException("Could not build the flink-dist image", e);
} finally {
temporaryFolder.delete();
}
}
+ private ImageFromDockerfile buildConfiguredImage(
+ String flinkDistName,
+ Path workersFile,
+ String baseImage) {
+ return new ImageFromDockerfile(
+ "flink-dist-configured")
+ .withDockerfileFromBuilder(
+ builder -> builder.from(baseImage)
+ .copy("workers", flinkDistName + "/conf/workers")
Review comment:
I think the arguments here should be reversed:
```
.copy(flinkDistName + "/conf/workers", "workers")
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14275: [FLINK-20432] Add timeouts to SQLClientSchemaRegistryITCase
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14275:
URL: https://github.com/apache/flink/pull/14275#issuecomment-736553908
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "30a8290a055ba720d2ffe39d759359b0cb62a0af",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10389",
"triggerID" : "30a8290a055ba720d2ffe39d759359b0cb62a0af",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 30a8290a055ba720d2ffe39d759359b0cb62a0af Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10389)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] kl0u commented on a change in pull request #14275: [FLINK-20432] Add timeouts to SQLClientSchemaRegistryITCase
Posted by GitBox <gi...@apache.org>.
kl0u commented on a change in pull request #14275:
URL: https://github.com/apache/flink/pull/14275#discussion_r535320520
##########
File path: flink-end-to-end-tests/flink-end-to-end-tests-common/src/main/java/org/apache/flink/tests/util/flink/FlinkContainer.java
##########
@@ -281,21 +280,19 @@ public FlinkContainer build() {
numTaskManagers,
logBackupDirectory.orElse(null));
} catch (Exception e) {
- throw new RuntimeException("Could not build the flink-dist image", e);
- } finally {
temporaryFolder.delete();
+ throw new RuntimeException("Could not build the flink-dist image", e);
}
}
private ImageFromDockerfile buildConfiguredImage(
- String flinkDistName,
Path workersFile,
String baseImage) {
return new ImageFromDockerfile(
"flink-dist-configured")
.withDockerfileFromBuilder(
builder -> builder.from(baseImage)
- .copy("workers", flinkDistName + "/conf/workers")
+ .copy("workers", "flink/conf/workers")
Review comment:
Ok after an offline discussion, this comment is not relevant anymore.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14275: [FLINK-20432] Add timeouts to SQLClientSchemaRegistryITCase
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14275:
URL: https://github.com/apache/flink/pull/14275#issuecomment-736553908
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "30a8290a055ba720d2ffe39d759359b0cb62a0af",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10389",
"triggerID" : "30a8290a055ba720d2ffe39d759359b0cb62a0af",
"triggerType" : "PUSH"
}, {
"hash" : "fadc71950d57be5c20ddb7a40089d06f6892bd21",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10491",
"triggerID" : "fadc71950d57be5c20ddb7a40089d06f6892bd21",
"triggerType" : "PUSH"
}, {
"hash" : "170410df4a117951b2d9950d66bef306c478e0e3",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "170410df4a117951b2d9950d66bef306c478e0e3",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 30a8290a055ba720d2ffe39d759359b0cb62a0af Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10389)
* fadc71950d57be5c20ddb7a40089d06f6892bd21 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10491)
* 170410df4a117951b2d9950d66bef306c478e0e3 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14275: [FLINK-20432] Add timeouts to SQLClientSchemaRegistryITCase
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14275:
URL: https://github.com/apache/flink/pull/14275#issuecomment-736553908
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "30a8290a055ba720d2ffe39d759359b0cb62a0af",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10389",
"triggerID" : "30a8290a055ba720d2ffe39d759359b0cb62a0af",
"triggerType" : "PUSH"
}, {
"hash" : "fadc71950d57be5c20ddb7a40089d06f6892bd21",
"status" : "CANCELED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10491",
"triggerID" : "fadc71950d57be5c20ddb7a40089d06f6892bd21",
"triggerType" : "PUSH"
}, {
"hash" : "170410df4a117951b2d9950d66bef306c478e0e3",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10498",
"triggerID" : "170410df4a117951b2d9950d66bef306c478e0e3",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* fadc71950d57be5c20ddb7a40089d06f6892bd21 Azure: [CANCELED](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10491)
* 170410df4a117951b2d9950d66bef306c478e0e3 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10498)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] kl0u commented on a change in pull request #14275: [FLINK-20432] Add timeouts to SQLClientSchemaRegistryITCase
Posted by GitBox <gi...@apache.org>.
kl0u commented on a change in pull request #14275:
URL: https://github.com/apache/flink/pull/14275#discussion_r535292580
##########
File path: flink-end-to-end-tests/flink-end-to-end-tests-common/src/main/java/org/apache/flink/tests/util/flink/FlinkContainer.java
##########
@@ -281,21 +280,19 @@ public FlinkContainer build() {
numTaskManagers,
logBackupDirectory.orElse(null));
} catch (Exception e) {
- throw new RuntimeException("Could not build the flink-dist image", e);
- } finally {
temporaryFolder.delete();
+ throw new RuntimeException("Could not build the flink-dist image", e);
}
}
private ImageFromDockerfile buildConfiguredImage(
- String flinkDistName,
Path workersFile,
String baseImage) {
return new ImageFromDockerfile(
"flink-dist-configured")
.withDockerfileFromBuilder(
builder -> builder.from(baseImage)
- .copy("workers", flinkDistName + "/conf/workers")
+ .copy("workers", "flink/conf/workers")
Review comment:
Here we are saying "copy the 'workers' file to the 'flink/conf/workers'" of the new image, right? What is in the 'workers' file? Shouldn't this be `/conf/workers`? Also on the master the args are reversed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14275: [FLINK-20432] Add timeouts to SQLClientSchemaRegistryITCase
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14275:
URL: https://github.com/apache/flink/pull/14275#issuecomment-736553908
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "30a8290a055ba720d2ffe39d759359b0cb62a0af",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10389",
"triggerID" : "30a8290a055ba720d2ffe39d759359b0cb62a0af",
"triggerType" : "PUSH"
}, {
"hash" : "fadc71950d57be5c20ddb7a40089d06f6892bd21",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "fadc71950d57be5c20ddb7a40089d06f6892bd21",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 30a8290a055ba720d2ffe39d759359b0cb62a0af Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10389)
* fadc71950d57be5c20ddb7a40089d06f6892bd21 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14275: [FLINK-20432] Add timeouts to SQLClientSchemaRegistryITCase
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14275:
URL: https://github.com/apache/flink/pull/14275#issuecomment-736553908
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "30a8290a055ba720d2ffe39d759359b0cb62a0af",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10389",
"triggerID" : "30a8290a055ba720d2ffe39d759359b0cb62a0af",
"triggerType" : "PUSH"
}, {
"hash" : "fadc71950d57be5c20ddb7a40089d06f6892bd21",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10491",
"triggerID" : "fadc71950d57be5c20ddb7a40089d06f6892bd21",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 30a8290a055ba720d2ffe39d759359b0cb62a0af Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10389)
* fadc71950d57be5c20ddb7a40089d06f6892bd21 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10491)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] dawidwys closed pull request #14275: [FLINK-20432] Add timeouts to SQLClientSchemaRegistryITCase
Posted by GitBox <gi...@apache.org>.
dawidwys closed pull request #14275:
URL: https://github.com/apache/flink/pull/14275
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14275: [FLINK-20432] Add timeouts to SQLClientSchemaRegistryITCase
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14275:
URL: https://github.com/apache/flink/pull/14275#issuecomment-736553908
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "30a8290a055ba720d2ffe39d759359b0cb62a0af",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10389",
"triggerID" : "30a8290a055ba720d2ffe39d759359b0cb62a0af",
"triggerType" : "PUSH"
}, {
"hash" : "fadc71950d57be5c20ddb7a40089d06f6892bd21",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10491",
"triggerID" : "fadc71950d57be5c20ddb7a40089d06f6892bd21",
"triggerType" : "PUSH"
}, {
"hash" : "170410df4a117951b2d9950d66bef306c478e0e3",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10498",
"triggerID" : "170410df4a117951b2d9950d66bef306c478e0e3",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 30a8290a055ba720d2ffe39d759359b0cb62a0af Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10389)
* fadc71950d57be5c20ddb7a40089d06f6892bd21 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10491)
* 170410df4a117951b2d9950d66bef306c478e0e3 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10498)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] dawidwys commented on a change in pull request #14275: [FLINK-20432] Add timeouts to SQLClientSchemaRegistryITCase
Posted by GitBox <gi...@apache.org>.
dawidwys commented on a change in pull request #14275:
URL: https://github.com/apache/flink/pull/14275#discussion_r535260432
##########
File path: flink-end-to-end-tests/flink-end-to-end-tests-common/src/main/java/org/apache/flink/tests/util/flink/FlinkContainer.java
##########
@@ -256,31 +258,83 @@ public FlinkContainer build() throws IOException {
.mapToObj(i -> "localhost")
.collect(Collectors.toList()));
- ImageFromDockerfile image = new ImageFromDockerfile("flink-dist", true)
- .withDockerfileFromBuilder(
- builder -> {
- builder.from("openjdk:" + getJavaVersionSuffix())
- .copy(flinkDistName, "flink")
- .copy(flinkDistName + "/conf/workers", "workers")
- .cmd(FLINK_BIN + "/start-cluster.sh && tail -f /dev/null")
- .build();
- }
- )
- .withFileFromPath("workers", workersFile)
- .withFileFromPath(flinkDistName, flinkDist);
+ // Building the docker image is split into two stages:
+ // 1. build a base image with an immutable flink-dist
+ // 2. based on the base image add any mutable files such as e.g. workers files
+ //
+ // This lets us save some time for archiving and copying big, immutable files
+ // between tests runs.
+ String baseImage = buildBaseImage(flinkDist, flinkDistName);
+ ImageFromDockerfile configuredImage = buildConfiguredImage(
+ flinkDistName,
+ workersFile,
+ baseImage);
Optional<Path> logBackupDirectory = DISTRIBUTION_LOG_BACKUP_DIRECTORY.get();
if (!logBackupDirectory.isPresent()) {
LOG.warn(
"Property {} not set, logs will not be backed up in case of test failures.",
DISTRIBUTION_LOG_BACKUP_DIRECTORY.getPropertyName());
}
- return new FlinkContainer(image, numTaskManagers, logBackupDirectory.orElse(null));
+ return new FlinkContainer(
+ configuredImage,
+ numTaskManagers,
+ logBackupDirectory.orElse(null));
+ } catch (Exception e) {
+ throw new RuntimeException("Could not build the flink-dist image", e);
} finally {
temporaryFolder.delete();
}
}
+ private ImageFromDockerfile buildConfiguredImage(
+ String flinkDistName,
+ Path workersFile,
+ String baseImage) {
+ return new ImageFromDockerfile(
+ "flink-dist-configured")
+ .withDockerfileFromBuilder(
+ builder -> builder.from(baseImage)
+ .copy("workers", flinkDistName + "/conf/workers")
Review comment:
Actually the order is correct. The first one is `source` the second `destination`. I am copying the `workers` file which I am including in the docker build context with `.withFileFromPath("workers", workersFile);` into the flink distribution folder to be picked up by the `start-cluster` script.
I did find two other bugs with the `workers` file when double checking.
1. First one is that I should've copied it over to `flink/conf/workers` instead of `flinkDistName + "/conf/workers"`
2. I should not delete the temporary folder in the finally block in the `build` method as the file is copied to the container during the start up of the FlinkContainer which is already after the `build` method.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot commented on pull request #14275: [FLINK-20432] Add timeouts to SQLClientSchemaRegistryITCase
Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #14275:
URL: https://github.com/apache/flink/pull/14275#issuecomment-736553908
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "30a8290a055ba720d2ffe39d759359b0cb62a0af",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "30a8290a055ba720d2ffe39d759359b0cb62a0af",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 30a8290a055ba720d2ffe39d759359b0cb62a0af UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14275: [FLINK-20432] Add timeouts to SQLClientSchemaRegistryITCase
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14275:
URL: https://github.com/apache/flink/pull/14275#issuecomment-736553908
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "30a8290a055ba720d2ffe39d759359b0cb62a0af",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10389",
"triggerID" : "30a8290a055ba720d2ffe39d759359b0cb62a0af",
"triggerType" : "PUSH"
}, {
"hash" : "fadc71950d57be5c20ddb7a40089d06f6892bd21",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10491",
"triggerID" : "fadc71950d57be5c20ddb7a40089d06f6892bd21",
"triggerType" : "PUSH"
}, {
"hash" : "170410df4a117951b2d9950d66bef306c478e0e3",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10498",
"triggerID" : "170410df4a117951b2d9950d66bef306c478e0e3",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 170410df4a117951b2d9950d66bef306c478e0e3 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10498)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] dawidwys commented on a change in pull request #14275: [FLINK-20432] Add timeouts to SQLClientSchemaRegistryITCase
Posted by GitBox <gi...@apache.org>.
dawidwys commented on a change in pull request #14275:
URL: https://github.com/apache/flink/pull/14275#discussion_r535260432
##########
File path: flink-end-to-end-tests/flink-end-to-end-tests-common/src/main/java/org/apache/flink/tests/util/flink/FlinkContainer.java
##########
@@ -256,31 +258,83 @@ public FlinkContainer build() throws IOException {
.mapToObj(i -> "localhost")
.collect(Collectors.toList()));
- ImageFromDockerfile image = new ImageFromDockerfile("flink-dist", true)
- .withDockerfileFromBuilder(
- builder -> {
- builder.from("openjdk:" + getJavaVersionSuffix())
- .copy(flinkDistName, "flink")
- .copy(flinkDistName + "/conf/workers", "workers")
- .cmd(FLINK_BIN + "/start-cluster.sh && tail -f /dev/null")
- .build();
- }
- )
- .withFileFromPath("workers", workersFile)
- .withFileFromPath(flinkDistName, flinkDist);
+ // Building the docker image is split into two stages:
+ // 1. build a base image with an immutable flink-dist
+ // 2. based on the base image add any mutable files such as e.g. workers files
+ //
+ // This lets us save some time for archiving and copying big, immutable files
+ // between tests runs.
+ String baseImage = buildBaseImage(flinkDist, flinkDistName);
+ ImageFromDockerfile configuredImage = buildConfiguredImage(
+ flinkDistName,
+ workersFile,
+ baseImage);
Optional<Path> logBackupDirectory = DISTRIBUTION_LOG_BACKUP_DIRECTORY.get();
if (!logBackupDirectory.isPresent()) {
LOG.warn(
"Property {} not set, logs will not be backed up in case of test failures.",
DISTRIBUTION_LOG_BACKUP_DIRECTORY.getPropertyName());
}
- return new FlinkContainer(image, numTaskManagers, logBackupDirectory.orElse(null));
+ return new FlinkContainer(
+ configuredImage,
+ numTaskManagers,
+ logBackupDirectory.orElse(null));
+ } catch (Exception e) {
+ throw new RuntimeException("Could not build the flink-dist image", e);
} finally {
temporaryFolder.delete();
}
}
+ private ImageFromDockerfile buildConfiguredImage(
+ String flinkDistName,
+ Path workersFile,
+ String baseImage) {
+ return new ImageFromDockerfile(
+ "flink-dist-configured")
+ .withDockerfileFromBuilder(
+ builder -> builder.from(baseImage)
+ .copy("workers", flinkDistName + "/conf/workers")
Review comment:
Actually the order is correct. The first one is `source` the second `destination`. I am copying the `workers` file which I am including in the docker build context with `.withFileFromPath("workers", workersFile);` into the flink distribution folder to be picked up by the `start-cluster` script.
I did find two bugs with the `workers` file when double checking.
1. First one is that I should've copied it over to `flink/conf/workers` instead of `flinkDistName + "/conf/workers"`
2. I should not delete the temporary folder in the finally block in the `build` method as the file is copied to the container during the start up of the FlinkContainer which is already after the `build` method.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14275: [FLINK-20432] Add timeouts to SQLClientSchemaRegistryITCase
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14275:
URL: https://github.com/apache/flink/pull/14275#issuecomment-736553908
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "30a8290a055ba720d2ffe39d759359b0cb62a0af",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10389",
"triggerID" : "30a8290a055ba720d2ffe39d759359b0cb62a0af",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 30a8290a055ba720d2ffe39d759359b0cb62a0af Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10389)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot commented on pull request #14275: [FLINK-20432] Add timeouts to SQLClientSchemaRegistryITCase
Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #14275:
URL: https://github.com/apache/flink/pull/14275#issuecomment-736532077
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
to review your pull request. We will use this comment to track the progress of the review.
## Automated Checks
Last check on commit 30a8290a055ba720d2ffe39d759359b0cb62a0af (Tue Dec 01 12:51:31 UTC 2020)
**Warnings:**
* No documentation files were touched! Remember to keep the Flink docs up to date!
<sub>Mention the bot in a comment to re-run the automated checks.</sub>
## Review Progress
* ❓ 1. The [description] looks good.
* ❓ 2. There is [consensus] that the contribution should go into to Flink.
* ❓ 3. Needs [attention] from.
* ❓ 4. The change fits into the overall [architecture].
* ❓ 5. Overall code [quality] is good.
Please see the [Pull Request Review Guide](https://flink.apache.org/contributing/reviewing-prs.html) for a full explanation of the review process.<details>
The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required <summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot approve description` to approve one or more aspects (aspects: `description`, `consensus`, `architecture` and `quality`)
- `@flinkbot approve all` to approve all aspects
- `@flinkbot approve-until architecture` to approve everything until `architecture`
- `@flinkbot attention @username1 [@username2 ..]` to require somebody's attention
- `@flinkbot disapprove architecture` to remove an approval you gave earlier
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] dawidwys commented on a change in pull request #14275: [FLINK-20432] Add timeouts to SQLClientSchemaRegistryITCase
Posted by GitBox <gi...@apache.org>.
dawidwys commented on a change in pull request #14275:
URL: https://github.com/apache/flink/pull/14275#discussion_r535299748
##########
File path: flink-end-to-end-tests/flink-end-to-end-tests-common/src/main/java/org/apache/flink/tests/util/flink/FlinkContainer.java
##########
@@ -281,21 +280,19 @@ public FlinkContainer build() {
numTaskManagers,
logBackupDirectory.orElse(null));
} catch (Exception e) {
- throw new RuntimeException("Could not build the flink-dist image", e);
- } finally {
temporaryFolder.delete();
+ throw new RuntimeException("Could not build the flink-dist image", e);
}
}
private ImageFromDockerfile buildConfiguredImage(
- String flinkDistName,
Path workersFile,
String baseImage) {
return new ImageFromDockerfile(
"flink-dist-configured")
.withDockerfileFromBuilder(
builder -> builder.from(baseImage)
- .copy("workers", flinkDistName + "/conf/workers")
+ .copy("workers", "flink/conf/workers")
Review comment:
Yes, the version on the master is wrong.
The workers file is one created manually above. See line 254.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org