zentol commented on a change in pull request #10106: [FLINK-11463][e2e] Design
the e2e java framework so that at least the Kafka streaming tests can run on it
URL: https://github.com/apache/flink/pull/10106#discussion_r343572818
##########
File path:
flink-end-to-end-tests/flink-end-to-end-tests-common/src/main/java/org/apache/flink/tests/util/FlinkDistribution.java
##########
@@ -123,20 +126,28 @@ public void afterTestSuccess() {
@Override
public void afterTestFailure() {
- logBackupDir.ifPresent(backupLocation -> {
+ if (logBackupDir != null) {
final UUID id = UUID.randomUUID();
- LOG.info("Backing up logs to {}/{}.", backupLocation,
id);
+ LOG.info("Backing up logs to {}/{}.", logBackupDir, id);
try {
- Files.createDirectories(backupLocation);
- FileUtils.copyDirectory(log.toFile(),
backupLocation.resolve(id.toString()).toFile());
+ Files.createDirectories(logBackupDir);
+ FileUtils.copyDirectory(log.toFile(),
logBackupDir.resolve(id.toString()).toFile());
} catch (IOException e) {
LOG.warn("An error occurred while backing up
logs.", e);
}
- });
-
+ }
afterTestSuccess();
}
+ /**
+ * Read the value of `rest.port` part in
FLINK_DIST_DIR/conf/flink-conf.yaml.
+ *
+ * @return the rest port which standalone Flink cluster will listen.
+ */
+ public int getRestPort() {
+ return defaultConfig.getInteger("rest.port", 8081);
Review comment:
As I said, tests will _eventually_ set this to 0, or more accurately, the
FlinkResource/FlinkDistribution implementation will do so to avoid port
conflicts.
Just likey all unit tests that rely on a `MiniCluster[Resource]` that have
the port set to 0.
Currently, the test in this PR relies on the default port (8081), purely
because the existing e2e tests make the same assumption. (As such there was no
immediate need to fix it)
This is _obviously_ problematic in the long run, but fine for now.
In contrast, relying on a setting in the configuration is just as
problematic (in the case of 0), but looks reasonable at a glance, making it a
worse approach.
I'd just replace this right her with a hard-coded `8081`, to make it very
obvious that this bit is problematic.
The _long-term_ solution is to set this port to 0 and parse the actually
used port from the log file.
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
