[Improve][Flink-Kubernetes-V2] add exception throw #3314
Conversation
|
Whether it is necessary for DeployRequest to be as complete as SubRequest, it seems that DeployRequest is imperfect at present, and many configurable parameters have not been passed in |
|
For ② I think it is OK, ① I am not sure if it is appropriate to make such a change |
|
cc @Al-assad |
| .getOption(KubernetesConfigOptions.CONTAINER_IMAGE_PULL_POLICY) | ||
| .map(_.toString) | ||
| .orElse(submitReq.k8sSubmitParam.imagePullPolicy) | ||
| val imagePullPolicy = Option( |
There was a problem hiding this comment.
imagePullPolicy is a Option[String] here which is not mandatory to have a value.
| @@ -103,12 +103,15 @@ object KubernetesApplicationClientV2 extends KubernetesClientV2Trait with Logger | |||
| .filter(str => StringUtils.isNotBlank(str)) | |||
| .getOrElse(return Left("Flink base image should not be empty")) | |||
|
|
|||
| val ingress = submitReq.k8sSubmitParam.ingressDefinition | |||
| val ingress = Option(submitReq.k8sSubmitParam.ingressDefinition) | |||
There was a problem hiding this comment.
ingress should be a Option[IngressDef] here.
| @@ -142,14 +145,17 @@ object KubernetesApplicationClientV2 extends KubernetesClientV2Trait with Logger | |||
| .orElse(submitReq.k8sSubmitParam.jobManagerMemory) | |||
| .getOrElse(KUBERNETES_JM_MEMORY_DEFAULT) | |||
|
|
|||
| val ephemeralStorage = Option(submitReq.k8sSubmitParam.jobManagerEphemeralStorage) | |||
There was a problem hiding this comment.
ephemeralStorage is is not a required parameter
Thank you for your reply and review . If the value is NULL, the return is null. There is no need to return an exception? |
|
I seem to understand what you mean. None doesn't affect the overall operation of the program, but Exception does, so those arguments can be None |
|
If so, I will shut this pr down |
In some places where it might be null, I add an exception throw
①[KubernetesApplicationClientV2.scala]
②[KubernetesSesionClientV2.scala]
What changes were proposed in this pull request
fix: #2882 and #2884
Brief change log
Verifying this change
This change is a trivial rework / code cleanup without any test coverage.
(or)
This change is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
Does this pull request potentially affect one of the following parts