subject:"\[GitHub\] \[flink\] vthinkxie commented on a change in pull request #13458\: FLINK\-18851 \[runtime\] Add checkpoint type to checkpoint history entries in Web UI"

[GitHub] [flink] vthinkxie commented on a change in pull request #13458: FLINK-18851 [runtime] Add checkpoint type to checkpoint history entries in Web UI

2020-10-28 Thread GitBox



vthinkxie commented on a change in pull request #13458:
URL: https://github.com/apache/flink/pull/13458#discussion_r513882504



##
File path: 
flink-runtime-web/web-dashboard/src/app/pages/job/checkpoints/detail/job-checkpoints-detail.component.ts
##
@@ -55,21 +59,38 @@ export class JobCheckpointsDetailComponent implements 
OnInit {
   }
 
   refresh() {
-this.isLoading = true;
-if (this.jobDetail && this.jobDetail.jid) {
-  this.jobService.loadCheckpointDetails(this.jobDetail.jid, 
this.checkPoint.id).subscribe(
-data => {
-  this.checkPointDetail = data;
-  this.isLoading = false;
-  this.cdr.markForCheck();
-},
-() => {
-  this.isLoading = false;
-  this.cdr.markForCheck();
-}
-  );
+  this.isLoading = true;
+  if (this.jobDetail && this.jobDetail.jid) {
+forkJoin([
+  this.jobService.loadCheckpointConfig(this.jobDetail.jid),
+  this.jobService.loadCheckpointDetails(this.jobDetail.jid, 
this.checkPoint.id)
+]).subscribe(
+  ([config, detail]) => {
+this.checkPointConfig = config;
+this.checkPointDetail = detail;
+if (this.checkPointDetail.checkpoint_type === 'CHECKPOINT') {
+  if (this.checkPointConfig.unaligned_checkpoints) {
+this.checkPointType = 'unaligned checkpoint';
+  } else {
+this.checkPointType = 'aligned checkpoint';
+  }
+} else if (this.checkPointDetail.checkpoint_type === 
'SYNC_SAVEPOINT') {
+  this.checkPointType = 'savepoint on cancel';
+} else if (this.checkPointDetail.checkpoint_type === 'SAVEPOINT') {
+  this.checkPointType = 'savepoint';
+} else {
+  this.checkPointType = '-';
+}
+this.isLoading = false;
+this.cdr.markForCheck();
+  },
+  () => {
+this.isLoading = false;
+this.cdr.markForCheck();
+  }
+);
+  }
 }

Review comment:
   Hi, I have a submit a commit here
   
https://github.com/vthinkxie/flink/commit/0a513eb9ac724b2a15cf4c95acdf22e5a9456d10
   could you have a look and apply this patch? There are still some format 
errors





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:
us...@infra.apache.org

[GitHub] [flink] vthinkxie commented on a change in pull request #13458: FLINK-18851 [runtime] Add checkpoint type to checkpoint history entries in Web UI

2020-10-28 Thread GitBox



vthinkxie commented on a change in pull request #13458:
URL: https://github.com/apache/flink/pull/13458#discussion_r513882504



##
File path: 
flink-runtime-web/web-dashboard/src/app/pages/job/checkpoints/detail/job-checkpoints-detail.component.ts
##
@@ -55,21 +59,38 @@ export class JobCheckpointsDetailComponent implements 
OnInit {
   }
 
   refresh() {
-this.isLoading = true;
-if (this.jobDetail && this.jobDetail.jid) {
-  this.jobService.loadCheckpointDetails(this.jobDetail.jid, 
this.checkPoint.id).subscribe(
-data => {
-  this.checkPointDetail = data;
-  this.isLoading = false;
-  this.cdr.markForCheck();
-},
-() => {
-  this.isLoading = false;
-  this.cdr.markForCheck();
-}
-  );
+  this.isLoading = true;
+  if (this.jobDetail && this.jobDetail.jid) {
+forkJoin([
+  this.jobService.loadCheckpointConfig(this.jobDetail.jid),
+  this.jobService.loadCheckpointDetails(this.jobDetail.jid, 
this.checkPoint.id)
+]).subscribe(
+  ([config, detail]) => {
+this.checkPointConfig = config;
+this.checkPointDetail = detail;
+if (this.checkPointDetail.checkpoint_type === 'CHECKPOINT') {
+  if (this.checkPointConfig.unaligned_checkpoints) {
+this.checkPointType = 'unaligned checkpoint';
+  } else {
+this.checkPointType = 'aligned checkpoint';
+  }
+} else if (this.checkPointDetail.checkpoint_type === 
'SYNC_SAVEPOINT') {
+  this.checkPointType = 'savepoint on cancel';
+} else if (this.checkPointDetail.checkpoint_type === 'SAVEPOINT') {
+  this.checkPointType = 'savepoint';
+} else {
+  this.checkPointType = '-';
+}
+this.isLoading = false;
+this.cdr.markForCheck();
+  },
+  () => {
+this.isLoading = false;
+this.cdr.markForCheck();
+  }
+);
+  }
 }

Review comment:
   Hi, I have a submit a commit here
   
https://github.com/vthinkxie/flink/commit/0a513eb9ac724b2a15cf4c95acdf22e5a9456d10
   could you have a look and apply this patch, there is still some format error





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:
us...@infra.apache.org

[GitHub] [flink] vthinkxie commented on a change in pull request #13458: FLINK-18851 [runtime] Add checkpoint type to checkpoint history entries in Web UI

2020-10-28 Thread GitBox



vthinkxie commented on a change in pull request #13458:
URL: https://github.com/apache/flink/pull/13458#discussion_r513374506



##
File path: 
flink-runtime-web/web-dashboard/src/app/pages/job/checkpoints/detail/job-checkpoints-detail.component.ts
##
@@ -55,21 +59,38 @@ export class JobCheckpointsDetailComponent implements 
OnInit {
   }
 
   refresh() {
-this.isLoading = true;
-if (this.jobDetail && this.jobDetail.jid) {
-  this.jobService.loadCheckpointDetails(this.jobDetail.jid, 
this.checkPoint.id).subscribe(
-data => {
-  this.checkPointDetail = data;
-  this.isLoading = false;
-  this.cdr.markForCheck();
-},
-() => {
-  this.isLoading = false;
-  this.cdr.markForCheck();
-}
-  );
+  this.isLoading = true;
+  if (this.jobDetail && this.jobDetail.jid) {
+forkJoin([
+  this.jobService.loadCheckpointConfig(this.jobDetail.jid),
+  this.jobService.loadCheckpointDetails(this.jobDetail.jid, 
this.checkPoint.id)
+]).subscribe(
+  ([config, detail]) => {
+this.checkPointConfig = config;
+this.checkPointDetail = detail;
+if (this.checkPointDetail.checkpoint_type === 'CHECKPOINT') {
+  if (this.checkPointConfig.unaligned_checkpoints) {
+this.checkPointType = 'unaligned checkpoint';
+  } else {
+this.checkPointType = 'aligned checkpoint';
+  }
+} else if (this.checkPointDetail.checkpoint_type === 
'SYNC_SAVEPOINT') {
+  this.checkPointType = 'savepoint on cancel';
+} else if (this.checkPointDetail.checkpoint_type === 'SAVEPOINT') {
+  this.checkPointType = 'savepoint';
+} else {
+  this.checkPointType = '-';
+}
+this.isLoading = false;
+this.cdr.markForCheck();
+  },
+  () => {
+this.isLoading = false;
+this.cdr.markForCheck();
+  }
+);
+  }
 }

Review comment:
   There are two extra spaces in each line, the others look good to me





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:
us...@infra.apache.org

[GitHub] [flink] vthinkxie commented on a change in pull request #13458: FLINK-18851 [runtime] Add checkpoint type to checkpoint history entries in Web UI

2020-10-26 Thread GitBox



vthinkxie commented on a change in pull request #13458:
URL: https://github.com/apache/flink/pull/13458#discussion_r512373747



##
File path: 
flink-runtime-web/web-dashboard/src/app/pages/job/checkpoints/detail/job-checkpoints-detail.component.ts
##
@@ -80,5 +97,9 @@ export class JobCheckpointsDetailComponent implements OnInit {
   this.cdr.markForCheck();
   this.refresh();
 });
+this.jobService.loadCheckpointConfig(this.jobDetail.jid).subscribe(config 
=> {

Review comment:
   these new lines could be removed after refactoring refresh func





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:
us...@infra.apache.org

[GitHub] [flink] vthinkxie commented on a change in pull request #13458: FLINK-18851 [runtime] Add checkpoint type to checkpoint history entries in Web UI

2020-10-26 Thread GitBox



vthinkxie commented on a change in pull request #13458:
URL: https://github.com/apache/flink/pull/13458#discussion_r512373566



##
File path: 
flink-runtime-web/web-dashboard/src/app/pages/job/checkpoints/detail/job-checkpoints-detail.component.ts
##
@@ -57,9 +60,23 @@ export class JobCheckpointsDetailComponent implements OnInit 
{
   refresh() {

Review comment:
   Hi @gm7y8 
   could you try to replace the refresh function with the codes below
   
   ```ts
   
 refresh() {
   this.isLoading = true;
   if (this.jobDetail && this.jobDetail.jid) {
 forkJoin([
   this.jobService.loadCheckpointConfig(this.jobDetail.jid),
   this.jobService.loadCheckpointDetails(this.jobDetail.jid, 
this.checkPoint.id)
 ]).subscribe(
   ([config, detail]) => {
 this.checkPointConfig = config;
 this.checkPointDetail = detail;
 if (this.checkPointDetail.checkpoint_type === 'CHECKPOINT') {
   if (this.checkPointConfig.unaligned_checkpoints) {
 this.checkPointType = 'unaligned checkpoint';
   } else {
 this.checkPointType = 'aligned checkpoint';
   }
 } else if (this.checkPointDetail.checkpoint_type === 
'SYNC_SAVEPOINT') {
   this.checkPointType = 'savepoint on cancel';
 } else if (this.checkPointDetail.checkpoint_type === 'SAVEPOINT') {
   this.checkPointType = 'savepoint';
 } else {
   this.checkPointType = '-';
 }
 this.isLoading = false;
 this.cdr.markForCheck();
   },
   () => {
 this.isLoading = false;
 this.cdr.markForCheck();
   }
 );
   }
 }
   
   ```





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:
us...@infra.apache.org

[GitHub] [flink] vthinkxie commented on a change in pull request #13458: FLINK-18851 [runtime] Add checkpoint type to checkpoint history entries in Web UI

2020-10-23 Thread GitBox



vthinkxie commented on a change in pull request #13458:
URL: https://github.com/apache/flink/pull/13458#discussion_r510680612



##
File path: 
flink-runtime-web/web-dashboard/src/app/pages/job/checkpoints/detail/job-checkpoints-detail.component.ts
##
@@ -80,5 +83,25 @@ export class JobCheckpointsDetailComponent implements OnInit 
{
   this.cdr.markForCheck();
   this.refresh();
 });
+this.jobService.loadCheckpointConfig(this.jobDetail.jid).subscribe(config 
=> {
+  this.checkPointConfig = config;
+  this.cdr.markForCheck();
+});
+  }
+
+  checkPointType(){

Review comment:
   it would be better to checkPointType as a value other than a function 
here
   then move the calculation to line 65





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:
us...@infra.apache.org

[GitHub] [flink] vthinkxie commented on a change in pull request #13458: FLINK-18851 [runtime] Add checkpoint type to checkpoint history entries in Web UI

2020-10-22 Thread GitBox



vthinkxie commented on a change in pull request #13458:
URL: https://github.com/apache/flink/pull/13458#discussion_r50994



##
File path: 
flink-runtime-web/web-dashboard/src/app/pages/job/checkpoints/detail/job-checkpoints-detail.component.html
##
@@ -21,6 +21,8 @@
 Path: {{checkPointDetail?.external_path || '-'}}
 
 Discarded: {{checkPointDetail?.discarded || '-'}}
+
+Checkpoint Type: {{checkPointDetail?.checkpoint_type === 
"CHECKPOINT" ? (checkPointConfig?.unaligned_checkpoints ? 'unaligned 
checkpoint' : 'aligned checkpoint') : (checkPointDetail?.checkpoint_type === 
"SYNC_SAVEPOINT" ? 'savepoint on cancel' : 'savepoint')|| '-'}}

Review comment:
   Hi @gm7y8 
   it would be better to move this calculation into the typescript file as a 
getter, and binding the getter here, since the calculation is a little too 
complex for the template file
   





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:
us...@infra.apache.org

[GitHub] [flink] vthinkxie commented on a change in pull request #13458: FLINK-18851 [runtime] Add checkpoint type to checkpoint history entries in Web UI

2020-10-22 Thread GitBox



vthinkxie commented on a change in pull request #13458:
URL: https://github.com/apache/flink/pull/13458#discussion_r50994



##
File path: 
flink-runtime-web/web-dashboard/src/app/pages/job/checkpoints/detail/job-checkpoints-detail.component.html
##
@@ -21,6 +21,8 @@
 Path: {{checkPointDetail?.external_path || '-'}}
 
 Discarded: {{checkPointDetail?.discarded || '-'}}
+
+Checkpoint Type: {{checkPointDetail?.checkpoint_type === 
"CHECKPOINT" ? (checkPointConfig?.unaligned_checkpoints ? 'unaligned 
checkpoint' : 'aligned checkpoint') : (checkPointDetail?.checkpoint_type === 
"SYNC_SAVEPOINT" ? 'savepoint on cancel' : 'savepoint')|| '-'}}

Review comment:
   Hi @gm7y8 
   it would be better to move this calculation into typescript file as a getter
   





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:
us...@infra.apache.org

[GitHub] [flink] vthinkxie commented on a change in pull request #13458: FLINK-18851 [runtime] Add checkpoint type to checkpoint history entries in Web UI

[GitHub] [flink] vthinkxie commented on a change in pull request #13458: FLINK-18851 [runtime] Add checkpoint type to checkpoint history entries in Web UI

[GitHub] [flink] vthinkxie commented on a change in pull request #13458: FLINK-18851 [runtime] Add checkpoint type to checkpoint history entries in Web UI

[GitHub] [flink] vthinkxie commented on a change in pull request #13458: FLINK-18851 [runtime] Add checkpoint type to checkpoint history entries in Web UI

[GitHub] [flink] vthinkxie commented on a change in pull request #13458: FLINK-18851 [runtime] Add checkpoint type to checkpoint history entries in Web UI

[GitHub] [flink] vthinkxie commented on a change in pull request #13458: FLINK-18851 [runtime] Add checkpoint type to checkpoint history entries in Web UI

[GitHub] [flink] vthinkxie commented on a change in pull request #13458: FLINK-18851 [runtime] Add checkpoint type to checkpoint history entries in Web UI

[GitHub] [flink] vthinkxie commented on a change in pull request #13458: FLINK-18851 [runtime] Add checkpoint type to checkpoint history entries in Web UI

8 matches

Site Navigation

Mail list logo

Footer information