mxm commented on code in PR #23406:
URL: https://github.com/apache/flink/pull/23406#discussion_r1328331551


##########
flink-core/src/main/java/org/apache/flink/configuration/RestOptions.java:
##########
@@ -80,6 +80,23 @@ public class RestOptions {
                     .withDescription(
                             "The address that should be used by clients to 
connect to the server. Attention: This option is respected only if the 
high-availability configuration is NONE.");
 
+    /** The path that should be used by clients to interact with the server. */
+    @Documentation.Section(Documentation.Sections.COMMON_HOST_PORT)
+    public static final ConfigOption<String> PATH =
+            key("rest.path")
+                    .stringType()
+                    .defaultValue("")
+                    .withDescription(
+                            "The path that should be used by clients to 
interact to the server which is accessible via URL.");
+
+    /** The protocol that should be used by clients to interact with the 
server. */
+    @Documentation.Section(Documentation.Sections.COMMON_HOST_PORT)
+    public static final ConfigOption<String> PROTOCOL =

Review Comment:
   Can this ever be anything else than `http` or `https`? I'm thinking that a 
bool would suffice here.



##########
flink-core/src/main/java/org/apache/flink/configuration/RestOptions.java:
##########
@@ -80,6 +80,23 @@ public class RestOptions {
                     .withDescription(
                             "The address that should be used by clients to 
connect to the server. Attention: This option is respected only if the 
high-availability configuration is NONE.");
 
+    /** The path that should be used by clients to interact with the server. */
+    @Documentation.Section(Documentation.Sections.COMMON_HOST_PORT)
+    public static final ConfigOption<String> PATH =
+            key("rest.path")
+                    .stringType()
+                    .defaultValue("")
+                    .withDescription(
+                            "The path that should be used by clients to 
interact to the server which is accessible via URL.");
+
+    /** The protocol that should be used by clients to interact with the 
server. */
+    @Documentation.Section(Documentation.Sections.COMMON_HOST_PORT)
+    public static final ConfigOption<String> PROTOCOL =
+            key("rest.protocol")
+                    .stringType()
+                    .defaultValue("http")
+                    .withDescription("The protocol to be used for rest 
endpoint");

Review Comment:
   The general pattern we follow in the Flink configuration is to control the 
protocol via the `ssl.enabled` flag, both for server and client. Do you think 
it would suffice to use this flag instead of adding another flag? 
   
   - 
https://nightlies.apache.org/flink/flink-docs-master/docs/deployment/security/security-ssl/#configuring-ssl
   - 
https://nightlies.apache.org/flink/flink-docs-master/docs/deployment/config/#security-ssl-rest-enabled



-- 
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-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to