[GitHub] [flink-kubernetes-operator] nowke commented on pull request #487: [FLINK-30411] Validate empty JmSpec and TmSpec

2022-12-30 Thread GitBox


nowke commented on PR #487:
URL: 
https://github.com/apache/flink-kubernetes-operator/pull/487#issuecomment-1368142154

   > I am still a bit torn on this PR. This adds quite a lot of complexity to 
the validation with some risks involved. Maybe we should just leave it as it is?
   
   Agreed on the added complexity, given the scope of the issue. Only 
additional benefit we get out of `processSpecFromConfig` from TM/JM utils is 
validating different combinations of `flinkConfiguration`, example: 
[https://github.com/apache/flink-kubernetes-operator/pull/487#discussion_r1059562798](https://github.com/apache/flink-kubernetes-operator/pull/487#discussion_r1059562798).
   
   I am totally fine closing the PR unless you have any alternative approach in 
mind.


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



[GitHub] [flink-kubernetes-operator] nowke commented on pull request #487: [FLINK-30411] Validate empty JmSpec and TmSpec

2022-12-20 Thread GitBox


nowke commented on PR #487:
URL: 
https://github.com/apache/flink-kubernetes-operator/pull/487#issuecomment-1360919023

   > @gyfora how do you think we should fix this?
   
   @morhidi @gyfora I added further validations with configs set through 
`flinkConfiguration`. It does make the validation logic a little complicated. 
Do you suggest any alternative approach?
   
   Besides helm validations, the deployment gets stuck in `UPGRADING` state due 
to `ClusterDeploymentException`. Should it fail the deployment instead upon 
encountering  `ClusterDeploymentException` a fixed number of times? Even after 
the validations in this PR, the deployment can still go into this state by 
setting insufficient memory.


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