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 2021/01/15 12:35:44 UTC

[GitHub] [flink] XComp opened a new pull request #14666: [FLINK-17085][kubernetes] Remove FlinkKubeClient.handleException

XComp opened a new pull request #14666:
URL: https://github.com/apache/flink/pull/14666


   ## What is the purpose of the change
   
   Code cleanup removing `FlinkKubeClient.handleException` as it doesn't add any value.
   
   ## Brief change log
   
   `handleException` was removed from `FlinkKubeClient` interface. The occurrences of this method were replaced by a more meaningful log message where necessary.
   
   ## Verifying this change
   
   This change is a trivial rework / code cleanup without any test coverage.
   
   ## Does this pull request potentially affect one of the following parts:
   
     - Dependencies (does it add or upgrade a dependency): no
     - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: no
     - The serializers: no
     - The runtime per-record code paths (performance sensitive): no
     - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn/Mesos, ZooKeeper: no
     - The S3 file system connector: no
   
   ## Documentation
   
     - Does this pull request introduce a new feature? no
     - If yes, how is the feature documented? not applicable
   


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

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



[GitHub] [flink] tillrohrmann commented on a change in pull request #14666: [FLINK-17085][kubernetes] Remove FlinkKubeClient.handleException

Posted by GitBox <gi...@apache.org>.
tillrohrmann commented on a change in pull request #14666:
URL: https://github.com/apache/flink/pull/14666#discussion_r559422674



##########
File path: flink-kubernetes/src/main/java/org/apache/flink/kubernetes/KubernetesClusterDescriptor.java
##########
@@ -118,7 +118,7 @@ public String getClusterDescription() {
                                         HighAvailabilityServicesUtils.AddressResolution
                                                 .TRY_ADDRESS_RESOLUTION)));
             } catch (Exception e) {
-                client.handleException(e);
+                LOG.error("An exception occurred while instantiating the RestClusterClient.", e);

Review comment:
       Hmm, maybe it was the intention to suppress all exceptions while the `LocalExecutor` is being shut down. Also, shouldn't the main thread log an uncaught exception? And if not, then we should probably add it there because who knows which other components follow the pattern to let exceptions bubble up to a point where they can be treated w/o logging it on the way.
   
   The problem with logging exceptions at several places is that the logs become  harder to understand/read.




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

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



[GitHub] [flink] tillrohrmann commented on a change in pull request #14666: [FLINK-17085][kubernetes] Remove FlinkKubeClient.handleException

Posted by GitBox <gi...@apache.org>.
tillrohrmann commented on a change in pull request #14666:
URL: https://github.com/apache/flink/pull/14666#discussion_r558462987



##########
File path: flink-kubernetes/src/main/java/org/apache/flink/kubernetes/KubernetesClusterDescriptor.java
##########
@@ -275,7 +275,9 @@ public void killCluster(String clusterId) throws FlinkException {
         try {
             client.stopAndCleanupCluster(clusterId);
         } catch (Exception e) {
-            client.handleException(e);
+            LOG.error(
+                    "An exception occurred while stopping the Kubernetes cluster and cleaning up.",
+                    e);

Review comment:
       Same here.

##########
File path: flink-kubernetes/src/main/java/org/apache/flink/kubernetes/KubernetesClusterDescriptor.java
##########
@@ -118,7 +118,7 @@ public String getClusterDescription() {
                                         HighAvailabilityServicesUtils.AddressResolution
                                                 .TRY_ADDRESS_RESOLUTION)));
             } catch (Exception e) {
-                client.handleException(e);
+                LOG.error("An exception occurred while instantiating the RestClusterClient.", e);

Review comment:
       Are you sure that we need these log statements? I think this is only required if the `RuntimeException` is nowhere logged.




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

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



[GitHub] [flink] XComp commented on a change in pull request #14666: [FLINK-17085][kubernetes] Remove FlinkKubeClient.handleException

Posted by GitBox <gi...@apache.org>.
XComp commented on a change in pull request #14666:
URL: https://github.com/apache/flink/pull/14666#discussion_r559443332



##########
File path: flink-kubernetes/src/main/java/org/apache/flink/kubernetes/KubernetesClusterDescriptor.java
##########
@@ -118,7 +118,7 @@ public String getClusterDescription() {
                                         HighAvailabilityServicesUtils.AddressResolution
                                                 .TRY_ADDRESS_RESOLUTION)));
             } catch (Exception e) {
-                client.handleException(e);
+                LOG.error("An exception occurred while instantiating the RestClusterClient.", e);

Review comment:
       Ok, thanks for clarification. I updated the code accordingly.




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

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



[GitHub] [flink] XComp commented on a change in pull request #14666: [FLINK-17085][kubernetes] Remove FlinkKubeClient.handleException

Posted by GitBox <gi...@apache.org>.
XComp commented on a change in pull request #14666:
URL: https://github.com/apache/flink/pull/14666#discussion_r558504220



##########
File path: flink-kubernetes/src/main/java/org/apache/flink/kubernetes/KubernetesClusterDescriptor.java
##########
@@ -275,7 +275,9 @@ public void killCluster(String clusterId) throws FlinkException {
         try {
             client.stopAndCleanupCluster(clusterId);
         } catch (Exception e) {
-            client.handleException(e);
+            LOG.error(
+                    "An exception occurred while stopping the Kubernetes cluster and cleaning up.",
+                    e);

Review comment:
       Here, I agree - I will revert it.




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

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



[GitHub] [flink] XComp commented on a change in pull request #14666: [FLINK-17085][kubernetes] Remove FlinkKubeClient.handleException

Posted by GitBox <gi...@apache.org>.
XComp commented on a change in pull request #14666:
URL: https://github.com/apache/flink/pull/14666#discussion_r558501302



##########
File path: flink-kubernetes/src/main/java/org/apache/flink/kubernetes/KubernetesClusterDescriptor.java
##########
@@ -118,7 +118,7 @@ public String getClusterDescription() {
                                         HighAvailabilityServicesUtils.AddressResolution
                                                 .TRY_ADDRESS_RESOLUTION)));
             } catch (Exception e) {
-                client.handleException(e);
+                LOG.error("An exception occurred while instantiating the RestClusterClient.", e);

Review comment:
       I thought so, too. But some call paths end up forwarding the `RuntimeException` up to the `main` method without any logging. Some paths swallowed the final exception without any logging (happened during closing/cancellation procedures like [LocalExecutor.closeSession(..)](https://github.com/apache/flink/blob/c6997c97c575d334679915c328792b8a3067cfb5/flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/gateway/local/LocalExecutor.java#L239)).
   Hence, I stepped back of this approach and for the safer option of logging it as it was discussed in the [initial PR discussion](https://github.com/apache/flink/pull/11427#discussion_r404221581).




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

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



[GitHub] [flink] flinkbot edited a comment on pull request #14666: [FLINK-17085][kubernetes] Remove FlinkKubeClient.handleException

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14666:
URL: https://github.com/apache/flink/pull/14666#issuecomment-760926129


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "f24833aedc768cdd4c3fa9426e7ed07814c82923",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12121",
       "triggerID" : "f24833aedc768cdd4c3fa9426e7ed07814c82923",
       "triggerType" : "PUSH"
     }, {
       "hash" : "8dc1342fec505ced26932fae17a394b456b2dac2",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "8dc1342fec505ced26932fae17a394b456b2dac2",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * f24833aedc768cdd4c3fa9426e7ed07814c82923 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12121) 
   * 8dc1342fec505ced26932fae17a394b456b2dac2 UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>


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

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



[GitHub] [flink] flinkbot commented on pull request #14666: [FLINK-17085][kubernetes] Remove FlinkKubeClient.handleException

Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #14666:
URL: https://github.com/apache/flink/pull/14666#issuecomment-760918458


   Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
   to review your pull request. We will use this comment to track the progress of the review.
   
   
   ## Automated Checks
   Last check on commit f24833aedc768cdd4c3fa9426e7ed07814c82923 (Fri Jan 15 12:38:30 UTC 2021)
   
   **Warnings:**
    * No documentation files were touched! Remember to keep the Flink docs up to date!
   
   
   <sub>Mention the bot in a comment to re-run the automated checks.</sub>
   ## Review Progress
   
   * ❓ 1. The [description] looks good.
   * ❓ 2. There is [consensus] that the contribution should go into to Flink.
   * ❓ 3. Needs [attention] from.
   * ❓ 4. The change fits into the overall [architecture].
   * ❓ 5. Overall code [quality] is good.
   
   Please see the [Pull Request Review Guide](https://flink.apache.org/contributing/reviewing-prs.html) for a full explanation of the review process.<details>
    The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot approve description` to approve one or more aspects (aspects: `description`, `consensus`, `architecture` and `quality`)
    - `@flinkbot approve all` to approve all aspects
    - `@flinkbot approve-until architecture` to approve everything until `architecture`
    - `@flinkbot attention @username1 [@username2 ..]` to require somebody's attention
    - `@flinkbot disapprove architecture` to remove an approval you gave earlier
   </details>


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

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



[GitHub] [flink] tillrohrmann closed pull request #14666: [FLINK-17085][kubernetes] Remove FlinkKubeClient.handleException

Posted by GitBox <gi...@apache.org>.
tillrohrmann closed pull request #14666:
URL: https://github.com/apache/flink/pull/14666


   


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

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



[GitHub] [flink] flinkbot edited a comment on pull request #14666: [FLINK-17085][kubernetes] Remove FlinkKubeClient.handleException

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14666:
URL: https://github.com/apache/flink/pull/14666#issuecomment-760926129


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "f24833aedc768cdd4c3fa9426e7ed07814c82923",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12121",
       "triggerID" : "f24833aedc768cdd4c3fa9426e7ed07814c82923",
       "triggerType" : "PUSH"
     }, {
       "hash" : "8dc1342fec505ced26932fae17a394b456b2dac2",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12130",
       "triggerID" : "8dc1342fec505ced26932fae17a394b456b2dac2",
       "triggerType" : "PUSH"
     }, {
       "hash" : "45efe4fc4f37db7d0ff5cb2e6b116a6c1a932dd9",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "45efe4fc4f37db7d0ff5cb2e6b116a6c1a932dd9",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 8dc1342fec505ced26932fae17a394b456b2dac2 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12130) 
   * 45efe4fc4f37db7d0ff5cb2e6b116a6c1a932dd9 UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>


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

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



[GitHub] [flink] flinkbot edited a comment on pull request #14666: [FLINK-17085][kubernetes] Remove FlinkKubeClient.handleException

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14666:
URL: https://github.com/apache/flink/pull/14666#issuecomment-760926129


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "f24833aedc768cdd4c3fa9426e7ed07814c82923",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12121",
       "triggerID" : "f24833aedc768cdd4c3fa9426e7ed07814c82923",
       "triggerType" : "PUSH"
     }, {
       "hash" : "8dc1342fec505ced26932fae17a394b456b2dac2",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12130",
       "triggerID" : "8dc1342fec505ced26932fae17a394b456b2dac2",
       "triggerType" : "PUSH"
     }, {
       "hash" : "45efe4fc4f37db7d0ff5cb2e6b116a6c1a932dd9",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12172",
       "triggerID" : "45efe4fc4f37db7d0ff5cb2e6b116a6c1a932dd9",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 45efe4fc4f37db7d0ff5cb2e6b116a6c1a932dd9 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12172) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>


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

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



[GitHub] [flink] flinkbot edited a comment on pull request #14666: [FLINK-17085][kubernetes] Remove FlinkKubeClient.handleException

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14666:
URL: https://github.com/apache/flink/pull/14666#issuecomment-760926129


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "f24833aedc768cdd4c3fa9426e7ed07814c82923",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12121",
       "triggerID" : "f24833aedc768cdd4c3fa9426e7ed07814c82923",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * f24833aedc768cdd4c3fa9426e7ed07814c82923 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12121) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>


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

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



[GitHub] [flink] flinkbot edited a comment on pull request #14666: [FLINK-17085][kubernetes] Remove FlinkKubeClient.handleException

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14666:
URL: https://github.com/apache/flink/pull/14666#issuecomment-760918458


   Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
   to review your pull request. We will use this comment to track the progress of the review.
   
   
   ## Automated Checks
   Last check on commit 45efe4fc4f37db7d0ff5cb2e6b116a6c1a932dd9 (Fri May 28 07:10:42 UTC 2021)
   
   **Warnings:**
    * No documentation files were touched! Remember to keep the Flink docs up to date!
   
   
   <sub>Mention the bot in a comment to re-run the automated checks.</sub>
   ## Review Progress
   
   * ❓ 1. The [description] looks good.
   * ❓ 2. There is [consensus] that the contribution should go into to Flink.
   * ❓ 3. Needs [attention] from.
   * ❓ 4. The change fits into the overall [architecture].
   * ❓ 5. Overall code [quality] is good.
   
   Please see the [Pull Request Review Guide](https://flink.apache.org/contributing/reviewing-prs.html) for a full explanation of the review process.<details>
    The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot approve description` to approve one or more aspects (aspects: `description`, `consensus`, `architecture` and `quality`)
    - `@flinkbot approve all` to approve all aspects
    - `@flinkbot approve-until architecture` to approve everything until `architecture`
    - `@flinkbot attention @username1 [@username2 ..]` to require somebody's attention
    - `@flinkbot disapprove architecture` to remove an approval you gave earlier
   </details>


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

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



[GitHub] [flink] flinkbot commented on pull request #14666: [FLINK-17085][kubernetes] Remove FlinkKubeClient.handleException

Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #14666:
URL: https://github.com/apache/flink/pull/14666#issuecomment-760926129


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "f24833aedc768cdd4c3fa9426e7ed07814c82923",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "f24833aedc768cdd4c3fa9426e7ed07814c82923",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * f24833aedc768cdd4c3fa9426e7ed07814c82923 UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>


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

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



[GitHub] [flink] flinkbot edited a comment on pull request #14666: [FLINK-17085][kubernetes] Remove FlinkKubeClient.handleException

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14666:
URL: https://github.com/apache/flink/pull/14666#issuecomment-760926129


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "f24833aedc768cdd4c3fa9426e7ed07814c82923",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12121",
       "triggerID" : "f24833aedc768cdd4c3fa9426e7ed07814c82923",
       "triggerType" : "PUSH"
     }, {
       "hash" : "8dc1342fec505ced26932fae17a394b456b2dac2",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12130",
       "triggerID" : "8dc1342fec505ced26932fae17a394b456b2dac2",
       "triggerType" : "PUSH"
     }, {
       "hash" : "45efe4fc4f37db7d0ff5cb2e6b116a6c1a932dd9",
       "status" : "PENDING",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12172",
       "triggerID" : "45efe4fc4f37db7d0ff5cb2e6b116a6c1a932dd9",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 8dc1342fec505ced26932fae17a394b456b2dac2 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12130) 
   * 45efe4fc4f37db7d0ff5cb2e6b116a6c1a932dd9 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12172) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>


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

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



[GitHub] [flink] flinkbot edited a comment on pull request #14666: [FLINK-17085][kubernetes] Remove FlinkKubeClient.handleException

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14666:
URL: https://github.com/apache/flink/pull/14666#issuecomment-760926129


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "f24833aedc768cdd4c3fa9426e7ed07814c82923",
       "status" : "PENDING",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12121",
       "triggerID" : "f24833aedc768cdd4c3fa9426e7ed07814c82923",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * f24833aedc768cdd4c3fa9426e7ed07814c82923 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12121) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>


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

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



[GitHub] [flink] flinkbot edited a comment on pull request #14666: [FLINK-17085][kubernetes] Remove FlinkKubeClient.handleException

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14666:
URL: https://github.com/apache/flink/pull/14666#issuecomment-760926129


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "f24833aedc768cdd4c3fa9426e7ed07814c82923",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12121",
       "triggerID" : "f24833aedc768cdd4c3fa9426e7ed07814c82923",
       "triggerType" : "PUSH"
     }, {
       "hash" : "8dc1342fec505ced26932fae17a394b456b2dac2",
       "status" : "PENDING",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12130",
       "triggerID" : "8dc1342fec505ced26932fae17a394b456b2dac2",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * f24833aedc768cdd4c3fa9426e7ed07814c82923 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12121) 
   * 8dc1342fec505ced26932fae17a394b456b2dac2 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12130) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>


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

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



[GitHub] [flink] flinkbot edited a comment on pull request #14666: [FLINK-17085][kubernetes] Remove FlinkKubeClient.handleException

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14666:
URL: https://github.com/apache/flink/pull/14666#issuecomment-760926129


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "f24833aedc768cdd4c3fa9426e7ed07814c82923",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12121",
       "triggerID" : "f24833aedc768cdd4c3fa9426e7ed07814c82923",
       "triggerType" : "PUSH"
     }, {
       "hash" : "8dc1342fec505ced26932fae17a394b456b2dac2",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12130",
       "triggerID" : "8dc1342fec505ced26932fae17a394b456b2dac2",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 8dc1342fec505ced26932fae17a394b456b2dac2 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12130) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>


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

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



[GitHub] [flink] XComp commented on a change in pull request #14666: [FLINK-17085][kubernetes] Remove FlinkKubeClient.handleException

Posted by GitBox <gi...@apache.org>.
XComp commented on a change in pull request #14666:
URL: https://github.com/apache/flink/pull/14666#discussion_r558501302



##########
File path: flink-kubernetes/src/main/java/org/apache/flink/kubernetes/KubernetesClusterDescriptor.java
##########
@@ -118,7 +118,7 @@ public String getClusterDescription() {
                                         HighAvailabilityServicesUtils.AddressResolution
                                                 .TRY_ADDRESS_RESOLUTION)));
             } catch (Exception e) {
-                client.handleException(e);
+                LOG.error("An exception occurred while instantiating the RestClusterClient.", e);

Review comment:
       I thought so, too. But some call paths end up forwarding the `RuntimeException` up to the `main` method without any logging. Some paths swallowed the final exception without any logging (happened during closing/cancellation procedures like [LocalExecutor.closeSession(..)](https://github.com/apache/flink/blob/c6997c97c575d334679915c328792b8a3067cfb5/flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/gateway/local/LocalExecutor.java#L239)).
   Hence, I stepped back from this approach and went for the safer option of logging it as it was discussed in the [initial PR discussion](https://github.com/apache/flink/pull/11427#discussion_r404221581).




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

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