nicoweidner commented on a change in pull request #17808:
URL: https://github.com/apache/flink/pull/17808#discussion_r750160260
##########
File path: flink-runtime-web/web-dashboard/src/app/interfaces/job-checkpoint.ts
##########
@@ -18,7 +18,7 @@
import { SafeAny } from './safe-any';
-export interface CheckPointInterface {
+export interface CheckPoint {
Review comment:
Hm... this should always be `Checkpoint`, right? Since this seems to be
widespread and was also spelled wrong before, fix if you like...
##########
File path: flink-runtime-web/web-dashboard/src/app/share/common/dagre/graph.ts
##########
@@ -40,10 +40,12 @@ export interface LayoutNodeOptions {
focused: boolean;
}
-export interface LayoutLink extends NodesItemLinkInterface {
- [key: string]: SafeAny;
+export interface LayoutLink extends NodesItemLink {
+ [key: string]: unknown;
- detail: SafeAny;
+ detail: {
+ [key: string]: unknown;
+ };
Review comment:
Isn't this just `Record<string, unknown>`?
##########
File path:
flink-runtime-web/web-dashboard/src/app/pages/job/checkpoints/subtask/job-checkpoints-subtask.component.ts
##########
@@ -40,31 +55,42 @@ import { deepFind } from 'utils';
changeDetection: ChangeDetectionStrategy.OnPush
})
export class JobCheckpointsSubtaskComponent implements OnInit, OnChanges {
- @Input() vertex: VerticesItemInterface;
- @Input() checkPointId: number;
- jobDetail: JobDetailCorrectInterface;
- subTaskCheckPoint: CheckPointSubTaskInterface;
- listOfSubTaskCheckPoint: Array<{ index: number; status: string }> = [];
- isLoading = true;
- sortName: string;
- sortValue: string;
+ @Input() public vertex: VerticesItem;
+ @Input() public checkPointId: number;
+
+ public jobDetail: JobDetailCorrect;
+ public subTaskCheckPoint: CheckPointSubTask;
+ public listOfSubTaskCheckPoint: SubTaskCheckPointStatisticsItem[] = [];
+ public isLoading = true;
+ public sortName: string;
+ public sortValue: string;
- sortAckTimestampFn = this.sortFn('ack_timestamp');
- sortEndToEndDurationFn = this.sortFn('end_to_end_duration');
- sortStateSizeFn = this.sortFn('state_size');
- sortCpSyncFn = this.sortFn('checkpoint.sync');
- sortCpAsyncFn = this.sortFn('checkpoint.async');
- sortAlignmentProcessedFn = this.sortFn('alignment.processed');
- sortAlignmentDurationFn = this.sortFn('alignment.duration');
- sortStartDelayFn = this.sortFn('start_delay');
- sortUnalignedCpFn = this.sortFn('unaligned_checkpoint');
+ public readonly sortAckTimestampFn = createSortFn(item =>
item.ack_timestamp);
+ public readonly sortEndToEndDurationFn = createSortFn(item =>
item.end_to_end_duration);
+ public readonly sortStateSizeFn = createSortFn(item => item.state_size);
+ public readonly sortCpSyncFn = createSortFn(item => item.checkpoint?.sync);
Review comment:
If anything is undefined, are we comparing deterministically? This
probably boils down to whether we compare the items in a deterministic order.
Do these sort functions support a result of 0 as "leave order unchanged"?
##########
File path:
flink-runtime-web/web-dashboard/src/app/pages/job/timeline/job-timeline.component.ts
##########
@@ -143,16 +189,18 @@ export class JobTimelineComponent implements
AfterViewInit, OnDestroy {
this.mainChartInstance.tooltip({
title: 'name'
});
- this.mainChartInstance.on('click', (e: SafeAny) => {
+ this.mainChartInstance.on('click', (e: { x: number; y: number }) => {
if (this.mainChartInstance.getSnapRecords(e).length) {
- const data = (this.mainChartInstance.getSnapRecords(e)[0] as
SafeAny)._origin;
+ const data = ((this.mainChartInstance.getSnapRecords(e)[0] as unknown)
as {
Review comment:
... can we really not avoid typing this as `number[0]` and then later on
typecasting it to access `._origin` after all?
##########
File path:
flink-runtime-web/web-dashboard/src/app/pages/submit/submit.component.ts
##########
@@ -71,24 +113,14 @@ export class SubmitComponent implements OnInit, OnDestroy {
);
}
- /**
- * Delete jar
- *
- * @param jar
- */
Review comment:
How is anyone supposed to understand what these functions do now,
without the docs?
##########
File path:
flink-runtime-web/web-dashboard/src/app/pages/job/overview/list/job-overview-list.component.ts
##########
@@ -16,12 +16,17 @@
* limitations under the License.
*/
-import { Component, EventEmitter, Input, Output, ChangeDetectionStrategy,
ElementRef } from '@angular/core';
+import { ChangeDetectionStrategy, Component, ElementRef, EventEmitter, Input,
Output } from '@angular/core';
import { NzTableSortFn } from 'ng-zorro-antd/table/src/table.types';
import { NodesItemCorrectInterface } from 'interfaces';
-import { deepFind } from 'utils';
+
+function createSortFn(
+ selector: (item: NodesItemCorrectInterface) => number | string | undefined
+): NzTableSortFn<NodesItemCorrectInterface> {
+ return (pre, next) => (selector(pre)! > selector(next)! ? 1 : -1);
Review comment:
Are we declaring that `selector` can return undefined, but then
asserting that it does not?
##########
File path:
flink-runtime-web/web-dashboard/src/app/share/common/dagre/svg-container.component.ts
##########
@@ -32,7 +32,8 @@ import * as d3 from 'd3';
import { select, Selection } from 'd3-selection';
import { zoom, ZoomBehavior } from 'd3-zoom';
-import { SafeAny } from 'interfaces';
+/* eslint-disable-next-line @typescript-eslint/no-explicit-any */
+export type SafeAny = any;
Review comment:
No way to remove this?
##########
File path: flink-runtime-web/web-dashboard/src/app/interfaces/job-checkpoint.ts
##########
@@ -113,7 +111,7 @@ export interface CheckPointTaskStatistics {
}
export interface CheckPointConfig {
- mode: SafeAny;
+ mode: 'exactly_once' | string;
Review comment:
Why the special role for `exactly_once`?
##########
File path: flink-runtime-web/web-dashboard/src/app/interfaces/job-detail.ts
##########
@@ -149,12 +147,12 @@ export interface NodesItemLinkInterface {
local_strategy?: string;
}
-export interface JobDetailCorrectInterface extends JobDetailInterface {
+export interface JobDetailCorrect extends JobDetail {
Review comment:
I am rather amused by interface names such as this one...
##########
File path:
flink-runtime-web/web-dashboard/src/app/share/common/dagre/svg-container.component.ts
##########
@@ -94,12 +86,12 @@ export class SvgContainerComponent implements OnInit,
AfterContentInit {
this.transform = `translate(${this.containerTransform.x}
,${this.containerTransform.y})scale(${this.containerTransform.k})`;
}
this.zoomEvent.emit(this.zoom);
- this.transformEvent.emit(this.containerTransform as SafeAny);
+ this.transformEvent.emit((this.containerTransform as unknown) as { x:
number; y: number; scale: number });
Review comment:
Why the weird double casting? I looked this one up as an example (since
there were so many cases), and `containerTransform` has properties `x`, `y`,
`k`. The latter might mean the same as `scale`, but here `scale` will just be
undefined, right?
--
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]
