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/05/13 14:50:31 UTC
[GitHub] [flink-kubernetes-operator] Aitozi commented on a diff in pull request #202: [FLINK-27483]Add session job config field
Aitozi commented on code in PR #202:
URL: https://github.com/apache/flink-kubernetes-operator/pull/202#discussion_r872450640
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/artifact/HttpArtifactFetcher.java:
##########
@@ -32,12 +37,24 @@ public class HttpArtifactFetcher implements ArtifactFetcher {
public static final HttpArtifactFetcher INSTANCE = new HttpArtifactFetcher();
@Override
- public File fetch(String uri, File targetDir) throws Exception {
+ public File fetch(String uri, Configuration flinkConfiguration, File targetDir)
+ throws Exception {
var start = System.currentTimeMillis();
URL url = new URL(uri);
+ HttpURLConnection conn = (HttpURLConnection) url.openConnection();
+
+ Map<String, String> clusterLevelHeader =
Review Comment:
we do not know this header is cluster level or job level here, what about naming it `headers` directly ?
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/crd/spec/AbstractFlinkSpec.java:
##########
@@ -40,4 +42,7 @@ public abstract class AbstractFlinkSpec {
* restart, change the number to anything other than the current value.
*/
private Long restartNonce;
+
+ /** Flink configuration overrides for the Flink deployment. */
Review Comment:
nit: the comment should be update: overrides for the Flink deployment or session job.
##########
docs/content/docs/operations/configuration.md:
##########
@@ -53,3 +53,11 @@ To learn more about metrics and logging configuration please refer to the dedica
## Operator Configuration Reference
{{< generated/kubernetes_operator_config_configuration >}}
+
+## Job Specific Configuration Reference
+Job specific configuration can be configured under `spec.job.flinkConfiguration` and it will override flink configurations defined in `flink-conf.yaml`.
+
+- For application clusters, `spec.job.flinkConfiguration` will be located in `FlinkDeployment` CustomResource.
+- For session clusters, configuring `spec.job.flinkConfiguration` in parent `FlinkDeployment` will be applied to all session jobs within the session cluster.
+You can also configure `spec.job.flinkConfiguration` in `FlinkSessionJob` CustomResource for a specific session job.
+The session job level configuration will override the parent session cluster's Flink configuration.
Review Comment:
I think we should let user know that the cluster level's flink config will not overridden by the session job's flinkConfiguration actually (I mean the session cluster's config). Personally, I think what sessionjob`spec.flinkConfiguration` defined, is not the **flink** Configuration, just some custom configs.
##########
docs/content/docs/operations/configuration.md:
##########
@@ -53,3 +53,11 @@ To learn more about metrics and logging configuration please refer to the dedica
## Operator Configuration Reference
{{< generated/kubernetes_operator_config_configuration >}}
+
+## Job Specific Configuration Reference
+Job specific configuration can be configured under `spec.job.flinkConfiguration` and it will override flink configurations defined in `flink-conf.yaml`.
+
+- For application clusters, `spec.job.flinkConfiguration` will be located in `FlinkDeployment` CustomResource.
Review Comment:
ditto
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/reconciler/sessionjob/FlinkSessionJobReconciler.java:
##########
@@ -83,6 +84,13 @@ public void reconcile(FlinkSessionJob flinkSessionJob, Context context) throws E
Configuration deployedConfig = configManager.getObserveConfig(flinkDepOptional.get());
+ // merge session job specific config
+ Map<String, String> sessionJobFlinkConfiguration =
Review Comment:
nit: maybe we could move this to the `FlinkConfigManager`
##########
docs/content/docs/operations/configuration.md:
##########
@@ -53,3 +53,11 @@ To learn more about metrics and logging configuration please refer to the dedica
## Operator Configuration Reference
{{< generated/kubernetes_operator_config_configuration >}}
+
+## Job Specific Configuration Reference
+Job specific configuration can be configured under `spec.job.flinkConfiguration` and it will override flink configurations defined in `flink-conf.yaml`.
+
+- For application clusters, `spec.job.flinkConfiguration` will be located in `FlinkDeployment` CustomResource.
+- For session clusters, configuring `spec.job.flinkConfiguration` in parent `FlinkDeployment` will be applied to all session jobs within the session cluster.
+You can also configure `spec.job.flinkConfiguration` in `FlinkSessionJob` CustomResource for a specific session job.
Review Comment:
A newline here
##########
docs/content/docs/operations/configuration.md:
##########
@@ -53,3 +53,11 @@ To learn more about metrics and logging configuration please refer to the dedica
## Operator Configuration Reference
{{< generated/kubernetes_operator_config_configuration >}}
+
+## Job Specific Configuration Reference
+Job specific configuration can be configured under `spec.job.flinkConfiguration` and it will override flink configurations defined in `flink-conf.yaml`.
Review Comment:
do you mean `spec.flinkConfiguration` ?
##########
docs/layouts/shortcodes/generated/kubernetes_operator_config_configuration.html:
##########
@@ -110,5 +110,11 @@
<td>String</td>
<td>The base dir to put the session job artifacts.</td>
</tr>
+ <tr>
+ <td><h5>kubernetes.operator.user.artifacts.http.header</h5></td>
+ <td style="word-wrap: break-word;">(none)</td>
+ <td>Map</td>
+ <td>Custom HTTP header for a Flink job. If configured in cluster level, headers will be applied to all jobs within the cluster. This field can also be configured under spec.job.flinkConfiguration for a specific session job within a session cluster. If configured at session job level, it will override the cluster level configuration. Expected format: headerKey1:headerValue1,headerKey2:headerValue2.</td>
Review Comment:
ditto
--
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