bgeng777 commented on a change in pull request #23:
URL:
https://github.com/apache/flink-kubernetes-operator/pull/23#discussion_r814535917
##########
File path:
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/observer/JobStatusObserver.java
##########
@@ -95,7 +95,7 @@ private JobStatus mergeJobStatus(
if (newStatus == null) {
newStatus = createJobStatus(newJob);
} else {
- newStatus.setState(newJob.getJobState().name());
+ newStatus.setState(JobState.valueOf(newJob.getJobState().name()));
Review comment:
Hi @gyfora thanks for sharing this point and I think about it as well
and I totally agree that there is some error handle missing here.
I discuss it a little in the
[jira](https://issues.apache.org/jira/browse/FLINK-26178). I think using the
job state enum should be reasonable as this state itself is an enum. I believe
the key is [FLINK-26139](https://issues.apache.org/jira/browse/FLINK-26139) has
not been resolved properly which makes the error handle in this part is
misleading. I would spend some time on FLINK-26139, hoping to propose a state
machine to describe the job state transition later.
This PR seems to be done a little early. We may finish it after we resolve
FLINK-26139. What's your advice?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
