You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@flink.apache.org by GitBox <gi...@apache.org> on 2022/03/24 08:31:19 UTC

[GitHub] [flink-kubernetes-operator] bgeng777 opened a new pull request #106: [FLINK-26832] Output more status info for JobObserver and BaseObserver

bgeng777 opened a new pull request #106:
URL: https://github.com/apache/flink-kubernetes-operator/pull/106


   - Make logger output current and next status in JobObserver
   - Make logger output current status in BaseObserver to help developers/users track status from k8s log


-- 
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: commits-unsubscribe@flink.apache.org

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

[GitHub] [flink-kubernetes-operator] bgeng777 commented on pull request #106: [FLINK-26832] Output more status info for JobObserver and BaseObserver

Posted by GitBox <gi...@apache.org>.
bgeng777 commented on pull request #106:
URL: https://github.com/apache/flink-kubernetes-operator/pull/106#issuecomment-1077406770


   cc @wangyang0918 @gyfora 


-- 
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: commits-unsubscribe@flink.apache.org

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

[GitHub] [flink-kubernetes-operator] gyfora commented on a change in pull request #106: [FLINK-26832] Output more status info for JobObserver and BaseObserver

Posted by GitBox <gi...@apache.org>.
gyfora commented on a change in pull request #106:
URL: https://github.com/apache/flink-kubernetes-operator/pull/106#discussion_r834111310



##########
File path: flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/observer/BaseObserver.java
##########
@@ -63,7 +63,7 @@ protected void observeJmDeployment(
         JobManagerDeploymentStatus previousJmStatus =
                 deploymentStatus.getJobManagerDeploymentStatus();
 
-        logger.info("Observing JobManager deployment status");
+        logger.info("Observing JobManager deployment with status: {}", previousJmStatus.name());

Review comment:
       we could probaby explicitly say `previous` here:
   ```
   Observing JobManager deployment. Previous status: {}
   ```




-- 
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: commits-unsubscribe@flink.apache.org

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

[GitHub] [flink-kubernetes-operator] gyfora merged pull request #106: [FLINK-26832] Output more status info for JobObserver and BaseObserver

Posted by GitBox <gi...@apache.org>.
gyfora merged pull request #106:
URL: https://github.com/apache/flink-kubernetes-operator/pull/106


   


-- 
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: commits-unsubscribe@flink.apache.org

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

[GitHub] [flink-kubernetes-operator] bgeng777 commented on a change in pull request #106: [FLINK-26832] Output more status info for JobObserver and BaseObserver

Posted by GitBox <gi...@apache.org>.
bgeng777 commented on a change in pull request #106:
URL: https://github.com/apache/flink-kubernetes-operator/pull/106#discussion_r834153888



##########
File path: flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/observer/JobObserver.java
##########
@@ -81,14 +81,20 @@ private boolean observeFlinkJobStatus(
             flinkAppStatus.getJobStatus().setState(JOB_STATE_UNKNOWN);
             return false;
         }
-
-        updateJobStatus(flinkAppStatus.getJobStatus(), new ArrayList<>(clusterJobStatuses));
-        logger.info("Job status successfully updated");
+        String targetJobStatus =
+                updateJobStatus(flinkAppStatus.getJobStatus(), new ArrayList<>(clusterJobStatuses));
+        logger.info(
+                "Job status successfully updated from {} to {}.",

Review comment:
       Updated.




-- 
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: commits-unsubscribe@flink.apache.org

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

[GitHub] [flink-kubernetes-operator] bgeng777 commented on a change in pull request #106: [FLINK-26832] Output more status info for JobObserver and BaseObserver

Posted by GitBox <gi...@apache.org>.
bgeng777 commented on a change in pull request #106:
URL: https://github.com/apache/flink-kubernetes-operator/pull/106#discussion_r834114857



##########
File path: flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/observer/BaseObserver.java
##########
@@ -63,7 +63,7 @@ protected void observeJmDeployment(
         JobManagerDeploymentStatus previousJmStatus =
                 deploymentStatus.getJobManagerDeploymentStatus();
 
-        logger.info("Observing JobManager deployment status");
+        logger.info("Observing JobManager deployment with status: {}", previousJmStatus.name());

Review comment:
       That is more accurate. Updated.




-- 
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: commits-unsubscribe@flink.apache.org

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

[GitHub] [flink-kubernetes-operator] gyfora commented on a change in pull request #106: [FLINK-26832] Output more status info for JobObserver and BaseObserver

Posted by GitBox <gi...@apache.org>.
gyfora commented on a change in pull request #106:
URL: https://github.com/apache/flink-kubernetes-operator/pull/106#discussion_r834144538



##########
File path: flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/observer/JobObserver.java
##########
@@ -81,14 +81,20 @@ private boolean observeFlinkJobStatus(
             flinkAppStatus.getJobStatus().setState(JOB_STATE_UNKNOWN);
             return false;
         }
-
-        updateJobStatus(flinkAppStatus.getJobStatus(), new ArrayList<>(clusterJobStatuses));
-        logger.info("Job status successfully updated");
+        String targetJobStatus =
+                updateJobStatus(flinkAppStatus.getJobStatus(), new ArrayList<>(clusterJobStatuses));
+        logger.info(
+                "Job status successfully updated from {} to {}.",

Review comment:
       To make this really nice we could add a branch:
   ```
   if (changed) {
    "Job status successfully updated from {} to {}."
    } else {
     "Job status ({state}) unchanged."
    }
   ```




-- 
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: commits-unsubscribe@flink.apache.org

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