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 2022/03/09 17:48:38 UTC
[GitHub] [flink] matriv opened a new pull request #19029: [FLINK-26418][runtime][test] Use java.io.tmpdir for tmpWorkingDir
matriv opened a new pull request #19029:
URL: https://github.com/apache/flink/pull/19029
Previously, the tmpWorkingDirectory was created in the current working
directory, and as a result there were directories created in the root
directories of the modules, i.e. `flink-table/flink-table-planner` which
were not cleaned up with `mvn clean`.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] matriv commented on a change in pull request #19029: [FLINK-26418][runtime][test] Use java.io.tmpdir for tmpWorkingDir
Posted by GitBox <gi...@apache.org>.
matriv commented on a change in pull request #19029:
URL: https://github.com/apache/flink/pull/19029#discussion_r823662489
##########
File path: flink-runtime/src/test/java/org/apache/flink/runtime/util/TestingTaskManagerRuntimeInfo.java
##########
@@ -38,7 +38,8 @@
public TestingTaskManagerRuntimeInfo() {
this(
new Configuration(),
- System.getProperty("java.io.tmpdir").split(",|" + File.pathSeparator));
+ EnvironmentInformation.getTemporaryFileDirectory()
Review comment:
@Myasuka what do you think ^^ ?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #19029: [FLINK-26418][runtime][test] Use java.io.tmpdir for tmpWorkingDir
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #19029:
URL: https://github.com/apache/flink/pull/19029#issuecomment-1063200095
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "90ce46910c3a9dd74fcf04412fe783473bfbde1f",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=32777",
"triggerID" : "90ce46910c3a9dd74fcf04412fe783473bfbde1f",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 90ce46910c3a9dd74fcf04412fe783473bfbde1f Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=32777)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] dawidwys commented on pull request #19029: [FLINK-26418][runtime][test] Use java.io.tmpdir for tmpWorkingDir
Posted by GitBox <gi...@apache.org>.
dawidwys commented on pull request #19029:
URL: https://github.com/apache/flink/pull/19029#issuecomment-1068930406
I am good as long as @Myasuka is. I'll merge it at the end of the day if there is no objection.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] slinkydeveloper commented on a change in pull request #19029: [FLINK-26418][runtime][test] Use java.io.tmpdir for tmpWorkingDir
Posted by GitBox <gi...@apache.org>.
slinkydeveloper commented on a change in pull request #19029:
URL: https://github.com/apache/flink/pull/19029#discussion_r823664403
##########
File path: flink-runtime/src/test/java/org/apache/flink/runtime/util/TestingTaskManagerRuntimeInfo.java
##########
@@ -38,7 +38,8 @@
public TestingTaskManagerRuntimeInfo() {
this(
new Configuration(),
- System.getProperty("java.io.tmpdir").split(",|" + File.pathSeparator));
+ EnvironmentInformation.getTemporaryFileDirectory()
Review comment:
Sounds too complicated IMO, let's keep it as it is in your commit
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] Myasuka commented on pull request #19029: [FLINK-26418][runtime][test] Use java.io.tmpdir for tmpWorkingDir
Posted by GitBox <gi...@apache.org>.
Myasuka commented on pull request #19029:
URL: https://github.com/apache/flink/pull/19029#issuecomment-1069090826
I compared the `flink-table` module logs in the [CI of this PR](https://dev.azure.com/apache-flink/apache-flink/_build/results?buildId=32777&view=artifacts&pathAsName=false&type=publishedArtifacts) with another [CI](https://dev.azure.com/apache-flink/apache-flink/_build/results?buildId=33106&view=artifacts&pathAsName=false&type=publishedArtifacts).
In the CI running, I can see tests which just use local working directory for RocksDB state backend:
~~~ java
Finished building RocksDB keyed state-backend at tmp_575da98d-5390-44fb-9e4c-2beaa05bb47c/job_ea6f78e05c817e20cdc6d80df39f7538_op_KeyedProcessOperator_7a62a488b6629c3cc1699b7c08f3c593__1_1__uuid_e9155b5d-a3cd-469e-b1f8-a080c9bda7aa.
~~~
On the other hand, with this PR, all RocksDB state backend would use folders under `/tmp`:
~~~java
Finished building RocksDB keyed state-backend at /tmp/tmp_8e74677c-a194-41df-9ce0-9209273c93de/job_955973f2518d3427585bedbd97fbb3ff_op_KeyedProcessOperator_7d3304e531c6834bde12a02fb1e9ad7f__1_1__uuid_5456c3c8-f69c-479a-9423-7583d474c6ea.
~~~
Maybe a junit temp dir is better, and I don't have strong objection for current solution.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] dawidwys merged pull request #19029: [FLINK-26418][runtime][test] Use java.io.tmpdir for tmpWorkingDir
Posted by GitBox <gi...@apache.org>.
dawidwys merged pull request #19029:
URL: https://github.com/apache/flink/pull/19029
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] Myasuka commented on pull request #19029: [FLINK-26418][runtime][test] Use java.io.tmpdir for tmpWorkingDir
Posted by GitBox <gi...@apache.org>.
Myasuka commented on pull request #19029:
URL: https://github.com/apache/flink/pull/19029#issuecomment-1063749872
I think this patch could resolve some problems, but I am not sure whether this fix could avoid to make all local test directories stay in the working directory.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot commented on pull request #19029: [FLINK-26418][runtime][test] Use java.io.tmpdir for tmpWorkingDir
Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #19029:
URL: https://github.com/apache/flink/pull/19029#issuecomment-1063200095
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "90ce46910c3a9dd74fcf04412fe783473bfbde1f",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "90ce46910c3a9dd74fcf04412fe783473bfbde1f",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 90ce46910c3a9dd74fcf04412fe783473bfbde1f UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] matriv commented on pull request #19029: [FLINK-26418][runtime][test] Use java.io.tmpdir for tmpWorkingDir
Posted by GitBox <gi...@apache.org>.
matriv commented on pull request #19029:
URL: https://github.com/apache/flink/pull/19029#issuecomment-1069154540
@dawidwys https://issues.apache.org/jira/browse/FLINK-26686
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] dawidwys commented on pull request #19029: [FLINK-26418][runtime][test] Use java.io.tmpdir for tmpWorkingDir
Posted by GitBox <gi...@apache.org>.
dawidwys commented on pull request #19029:
URL: https://github.com/apache/flink/pull/19029#issuecomment-1069130017
I think this PR makes it better than it was before. Let's create a follow-up ticket for using a junit temporary directory in `TestingTaskManagerRuntimeInfo`. Do you mind creating one @matriv ? I will merge this PR as is.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] slinkydeveloper commented on a change in pull request #19029: [FLINK-26418][runtime][test] Use java.io.tmpdir for tmpWorkingDir
Posted by GitBox <gi...@apache.org>.
slinkydeveloper commented on a change in pull request #19029:
URL: https://github.com/apache/flink/pull/19029#discussion_r823457633
##########
File path: flink-runtime/src/test/java/org/apache/flink/runtime/util/TestingTaskManagerRuntimeInfo.java
##########
@@ -38,7 +38,8 @@
public TestingTaskManagerRuntimeInfo() {
this(
new Configuration(),
- System.getProperty("java.io.tmpdir").split(",|" + File.pathSeparator));
+ EnvironmentInformation.getTemporaryFileDirectory()
Review comment:
We should use junit temp dir here instead, but that's ok for now if it fixes the issue
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] matriv commented on pull request #19029: [FLINK-26418][runtime][test] Use java.io.tmpdir for tmpWorkingDir
Posted by GitBox <gi...@apache.org>.
matriv commented on pull request #19029:
URL: https://github.com/apache/flink/pull/19029#issuecomment-1069117115
@Myasuka
Then we would need to remove that constructor completely and let every test use:
```
public TestingTaskManagerRuntimeInfo(
Configuration configuration,
String[] tmpDirectories,
String taskManagerExternalAddress,
File tmpWorkingDirectory) {
this.configuration = configuration;
this.tmpDirectories = tmpDirectories;
this.taskManagerExternalAddress = taskManagerExternalAddress;
this.tmpWorkingDirectory = tmpWorkingDirectory;
}
```
where the test class will supply a junit temp folder instead.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] dawidwys commented on pull request #19029: [FLINK-26418][runtime][test] Use java.io.tmpdir for tmpWorkingDir
Posted by GitBox <gi...@apache.org>.
dawidwys commented on pull request #19029:
URL: https://github.com/apache/flink/pull/19029#issuecomment-1063739269
cc @Myasuka
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #19029: [FLINK-26418][runtime][test] Use java.io.tmpdir for tmpWorkingDir
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #19029:
URL: https://github.com/apache/flink/pull/19029#issuecomment-1063200095
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "90ce46910c3a9dd74fcf04412fe783473bfbde1f",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=32777",
"triggerID" : "90ce46910c3a9dd74fcf04412fe783473bfbde1f",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 90ce46910c3a9dd74fcf04412fe783473bfbde1f Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=32777)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@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.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] matriv commented on a change in pull request #19029: [FLINK-26418][runtime][test] Use java.io.tmpdir for tmpWorkingDir
Posted by GitBox <gi...@apache.org>.
matriv commented on a change in pull request #19029:
URL: https://github.com/apache/flink/pull/19029#discussion_r823661973
##########
File path: flink-runtime/src/test/java/org/apache/flink/runtime/util/TestingTaskManagerRuntimeInfo.java
##########
@@ -38,7 +38,8 @@
public TestingTaskManagerRuntimeInfo() {
this(
new Configuration(),
- System.getProperty("java.io.tmpdir").split(",|" + File.pathSeparator));
+ EnvironmentInformation.getTemporaryFileDirectory()
Review comment:
I thought about it, but this is a helper class, it's not a "junit" specific class, and I thought we shouldn't use `TemporaryFolder` directly.
Another idea is to remove ctors with no `tmpWorkingDir` so it's a mandatory argument, and all consumers should be responsible to pass a directory path there, so all the tests using it could pass a junit `TemporaryFolder.newFolder()`.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] matriv commented on pull request #19029: [FLINK-26418][runtime][test] Use java.io.tmpdir for tmpWorkingDir
Posted by GitBox <gi...@apache.org>.
matriv commented on pull request #19029:
URL: https://github.com/apache/flink/pull/19029#issuecomment-1068918786
@Myasuka @dawidwys Do you have any objections on this? otherwise I'd say to go ahead and merge it.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org