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 2022/05/22 15:54:09 UTC

[GitHub] [flink] zoltar9264 opened a new pull request, #19789: [FLINK-27734][runtime-web] fix checkpoint interval in WebUI

zoltar9264 opened a new pull request, #19789:
URL: https://github.com/apache/flink/pull/19789

   ## What is the purpose of the change
   
   fix checkpoint interval in WebUI , which is described in [FLINK-27734](https://issues.apache.org/jira/browse/FLINK-27734)
   
   ## 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, 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? (no)
   


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

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


[GitHub] [flink] zoltar9264 commented on pull request #19789: [FLINK-27734][web] Fix checkpoint interval in WebUI

Posted by GitBox <gi...@apache.org>.
zoltar9264 commented on PR #19789:
URL: https://github.com/apache/flink/pull/19789#issuecomment-1149727152

   Thanks @Myasuka , done.
   > LGTM, could you please squash all commits into one?


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

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


[GitHub] [flink] zoltar9264 commented on a diff in pull request #19789: [FLINK-27734][web] Fix checkpoint interval in WebUI

Posted by GitBox <gi...@apache.org>.
zoltar9264 commented on code in PR #19789:
URL: https://github.com/apache/flink/pull/19789#discussion_r890102137


##########
flink-runtime-web/web-dashboard/.eslintrc.js:
##########
@@ -126,7 +126,9 @@ module.exports = {
     {
       files: ['*.html'],
       extends: ['plugin:@angular-eslint/template/recommended'],
-      rules: {}
+      rules: {
+        '@angular-eslint/template/eqeqeq': 'off'

Review Comment:
   Thanks for your suggestion @Myasuka !



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

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


[GitHub] [flink] zoltar9264 commented on a diff in pull request #19789: [FLINK-27734][web] Fix checkpoint interval in WebUI

Posted by GitBox <gi...@apache.org>.
zoltar9264 commented on code in PR #19789:
URL: https://github.com/apache/flink/pull/19789#discussion_r893027607


##########
flink-runtime-web/web-dashboard/src/app/pages/job/checkpoints/job-checkpoints.component.html:
##########
@@ -500,10 +500,10 @@
           </tr>
           <tr>
             <td>Interval</td>
-            <td *ngIf="checkPointConfig['interval'] === '0x7fffffffffffffff'">
+            <td *ngIf="checkPointConfig['interval'] == '0x7fffffffffffffff'">

Review Comment:
   Thanks @yangjunhan ! I will hive a try.



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

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


[GitHub] [flink] Myasuka commented on a diff in pull request #19789: [FLINK-27734][web] Fix checkpoint interval in WebUI

Posted by GitBox <gi...@apache.org>.
Myasuka commented on code in PR #19789:
URL: https://github.com/apache/flink/pull/19789#discussion_r886914800


##########
flink-runtime-web/web-dashboard/.eslintrc.js:
##########
@@ -126,7 +126,9 @@ module.exports = {
     {
       files: ['*.html'],
       extends: ['plugin:@angular-eslint/template/recommended'],
-      rules: {}
+      rules: {
+        '@angular-eslint/template/eqeqeq': 'off'

Review Comment:
   I see there existing other places to use `===` in the web code, can we make the rule as `warn` instead of `off` as [doc](https://cn.eslint.org/docs/user-guide/configuring#configuring-rules) said.



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

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


[GitHub] [flink] yangjunhan commented on a diff in pull request #19789: [FLINK-27734][web] Fix checkpoint interval in WebUI

Posted by GitBox <gi...@apache.org>.
yangjunhan commented on code in PR #19789:
URL: https://github.com/apache/flink/pull/19789#discussion_r893012502


##########
flink-runtime-web/web-dashboard/.eslintrc.js:
##########
@@ -126,7 +126,9 @@ module.exports = {
     {
       files: ['*.html'],
       extends: ['plugin:@angular-eslint/template/recommended'],
-      rules: {}
+      rules: {
+        '@angular-eslint/template/eqeqeq': 'warn'

Review Comment:
   Don't do this. You should prefer **Strict Equality Comparison** over **Abstract Equality Comparison** for being type-safe.



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

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


[GitHub] [flink] Myasuka merged pull request #19789: [FLINK-27734][web] Fix checkpoint interval in WebUI

Posted by GitBox <gi...@apache.org>.
Myasuka merged PR #19789:
URL: https://github.com/apache/flink/pull/19789


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

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


[GitHub] [flink] zoltar9264 commented on pull request #19789: [FLINK-27734][web] Fix checkpoint interval in WebUI

Posted by GitBox <gi...@apache.org>.
zoltar9264 commented on PR #19789:
URL: https://github.com/apache/flink/pull/19789#issuecomment-1150692967

   Thanks @yangjunhan and @Myasuka , I tried  this proposal and it works. I had updated this PR.


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

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


[GitHub] [flink] flinkbot commented on pull request #19789: [FLINK-27734][runtime-web] fix checkpoint interval in WebUI

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

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "7949be0eb75fb1ce48821e2298de1b3551dae197",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "7949be0eb75fb1ce48821e2298de1b3551dae197",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 7949be0eb75fb1ce48821e2298de1b3551dae197 UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@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.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

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


[GitHub] [flink] zoltar9264 commented on pull request #19789: [FLINK-27734][web] Fix checkpoint interval in WebUI

Posted by GitBox <gi...@apache.org>.
zoltar9264 commented on PR #19789:
URL: https://github.com/apache/flink/pull/19789#issuecomment-1148646489

   @flinkbot run azure


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

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


[GitHub] [flink] yangjunhan commented on a diff in pull request #19789: [FLINK-27734][web] Fix checkpoint interval in WebUI

Posted by GitBox <gi...@apache.org>.
yangjunhan commented on code in PR #19789:
URL: https://github.com/apache/flink/pull/19789#discussion_r893012727


##########
flink-runtime-web/web-dashboard/src/app/pages/job/checkpoints/job-checkpoints.component.html:
##########
@@ -500,10 +500,10 @@
           </tr>
           <tr>
             <td>Interval</td>
-            <td *ngIf="checkPointConfig['interval'] === '0x7fffffffffffffff'">
+            <td *ngIf="checkPointConfig['interval'] == '0x7fffffffffffffff'">
               Periodic checkpoints disabled
             </td>
-            <td *ngIf="checkPointConfig['interval'] !== '0x7fffffffffffffff'">
+            <td *ngIf="checkPointConfig['interval'] != '0x7fffffffffffffff'">

Review Comment:
   Same as above.



##########
flink-runtime-web/web-dashboard/.eslintrc.js:
##########
@@ -126,7 +126,9 @@ module.exports = {
     {
       files: ['*.html'],
       extends: ['plugin:@angular-eslint/template/recommended'],
-      rules: {}
+      rules: {
+        '@angular-eslint/template/eqeqeq': 'warn'

Review Comment:
   Don't do this. You shouldn't allow Abstract Equality Comparison, and always prefer Strict Equality Comparison for being type-safe.



##########
flink-runtime-web/web-dashboard/src/app/pages/job/checkpoints/job-checkpoints.component.html:
##########
@@ -500,10 +500,10 @@
           </tr>
           <tr>
             <td>Interval</td>
-            <td *ngIf="checkPointConfig['interval'] === '0x7fffffffffffffff'">
+            <td *ngIf="checkPointConfig['interval'] == '0x7fffffffffffffff'">

Review Comment:
   ```suggestion
               <td *ngIf="checkPointConfig['interval'] === disabledInterval">
   ```
   
   You can utilize the Angular property binding to fix this issue. 
   
   Define `disabledInterval = 0x7fffffffffffffff` in the `JobCheckpointsComponent` class. 



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

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


[GitHub] [flink] Myasuka commented on a diff in pull request #19789: [FLINK-27734][web] Fix checkpoint interval in WebUI

Posted by GitBox <gi...@apache.org>.
Myasuka commented on code in PR #19789:
URL: https://github.com/apache/flink/pull/19789#discussion_r893023170


##########
flink-runtime-web/web-dashboard/src/app/pages/job/checkpoints/job-checkpoints.component.html:
##########
@@ -500,10 +500,10 @@
           </tr>
           <tr>
             <td>Interval</td>
-            <td *ngIf="checkPointConfig['interval'] === '0x7fffffffffffffff'">
+            <td *ngIf="checkPointConfig['interval'] == '0x7fffffffffffffff'">

Review Comment:
   Thanks for your suggestion! And luckily that we did not make this code merged. @zoltar9264 could you try this proposal and update this PR if valid?



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

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