zentol commented on a change in pull request #14014:
URL: https://github.com/apache/flink/pull/14014#discussion_r540524925



##########
File path: 
flink-core/src/main/java/org/apache/flink/configuration/CoreOptions.java
##########
@@ -218,6 +218,17 @@
                .withDescription("Defines the directory where the Flink logs 
are saved. It has to be an absolute path." +
                        " (Defaults to the log directory under Flink’s home)");
 
+       /**
+        * This options is here only for documentation generation, it is only

Review comment:
       It would be good to explicitly link to `config.sh#KEY_ENV_PID_DIR `and 
`config.sh#DEFAULT_ENV_PID_DIR`.

##########
File path: 
flink-core/src/main/java/org/apache/flink/configuration/CoreOptions.java
##########
@@ -218,6 +218,17 @@
                .withDescription("Defines the directory where the Flink logs 
are saved. It has to be an absolute path." +
                        " (Defaults to the log directory under Flink’s home)");
 
+       /**
+        * This options is here only for documentation generation, it is only
+        * evaluated in the shell scripts.
+        */
+       public static final ConfigOption<String> FLINK_APP_PID_DIR = 
ConfigOptions

Review comment:
       ```suggestion
        public static final ConfigOption<String> FLINK_PID_DIR = ConfigOptions
   ```

##########
File path: 
flink-core/src/main/java/org/apache/flink/configuration/CoreOptions.java
##########
@@ -218,6 +218,17 @@
                .withDescription("Defines the directory where the Flink logs 
are saved. It has to be an absolute path." +
                        " (Defaults to the log directory under Flink’s home)");
 
+       /**
+        * This options is here only for documentation generation, it is only
+        * evaluated in the shell scripts.
+        */
+       public static final ConfigOption<String> FLINK_APP_PID_DIR = 
ConfigOptions
+               .key("env.pid.dir")
+               .noDefaultValue()

Review comment:
       the default is "/tmp", which we should document

##########
File path: 
flink-core/src/main/java/org/apache/flink/configuration/CoreOptions.java
##########
@@ -218,6 +218,17 @@
                .withDescription("Defines the directory where the Flink logs 
are saved. It has to be an absolute path." +
                        " (Defaults to the log directory under Flink’s home)");
 
+       /**
+        * This options is here only for documentation generation, it is only
+        * evaluated in the shell scripts.
+        */
+       public static final ConfigOption<String> FLINK_APP_PID_DIR = 
ConfigOptions
+               .key("env.pid.dir")
+               .noDefaultValue()
+               .withDescription(
+                       "Defines the directory where the 
flink-<host>-<process>.pid files are saved."
+                               + "(Defaults to the system tmp directory)");

Review comment:
       ```suggestion
                        "Defines the directory where the 
flink-<host>-<process>.pid files are saved.");
   ```




----------------------------------------------------------------
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]


Reply via email to