[FLINK-24737][runtime-web] Update outdated web dependencies #17711

yangjunhan · 2021-11-08T08:23:49Z

What is the purpose of the change

Update outdated web dependencies and dev-dependencies.
https://issues.apache.org/jira/browse/FLINK-24737

Brief change log

Most dependencies have been updated to the latest versions except Angular (v13) and its dependent Rxjs and TypeScript, etc. According to npm audit, the remaining unresolved dependency vulnerabilities are mainly caused by Angular and should be fixed while bumping it to v13.

Note: Bumping the Angular framework to v13 should be in another PR once NG-Zorro-Antd releases their compatible v13.

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

flinkbot · 2021-11-08T08:26:26Z

CI report:

01cc7a5 Azure: SUCCESS

Bot commands

The @flinkbot bot supports the following commands:

@flinkbot run azure re-run the last Azure build

flinkbot · 2021-11-08T08:29:06Z

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 80fbcfa (Mon Nov 08 08:29:05 UTC 2021)

Warnings:

No documentation files were touched! Remember to keep the Flink docs up to date!

_{Mention the bot in a comment to re-run the automated checks.}

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 for a full explanation of the review process.

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

Bot commands

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

flink-runtime-web/web-dashboard/package.json

MartijnVisser · 2021-12-10T12:17:36Z

@yangjunhan Can you have a look at the failing build and fix it?

yangjunhan · 2021-12-15T06:23:23Z

Can you have a look at the failing build and fix it?

@MartijnVisser Conflicts have been addressed.

MartijnVisser · 2021-12-15T07:08:26Z

Can you have a look at the failing build and fix it?

@MartijnVisser Conflicts have been addressed.

Thanks @yangjunhan The CI unfortunately still fails and reports:

[INFO] npm ERR! @jest/types not accessible from pretty-format

yangjunhan · 2021-12-15T09:01:51Z

@flinkbot run azure

MartijnVisser · 2021-12-15T13:30:35Z

@flinkbot run azure

MartijnVisser · 2021-12-15T18:25:13Z

@Airblader Could you have a look?

Airblader

Thanks for the PR and bringing the dependencies more up to date, @yangjunhan. Overall this looks good to me, but I have a couple questions.

However, it seems that now when making commits unrelated to the UI, there is output that didn't use to exist:

$ git commit …

> flink-dashboard@2.0.0 lint-staged
> lint-staged

→ No staged files match any configured task.

I think this will confuse the majority of developers as most do not work on the UI. Can we do something to restore the old behavior of not producing output unless the commit contains changes to the UI?

Airblader · 2021-12-16T07:47:08Z

flink-runtime-web/web-dashboard/.husky/pre-commit

@@ -0,0 +1,4 @@
+#!/bin/sh


Please use the more portable

#!/usr/bin/env bash

Airblader · 2021-12-16T07:54:05Z

flink-runtime-web/web-dashboard/src/app/share/common/dagre/svg-container.component.ts

+      .on('zoom', ({ transform }: SafeAny) => {
+        const { x, y, k } = transform;
+        this.zoom = k;
+        this.zoomEvent.emit(k);


Why do we need the additional emit here?

This seemed to be a mistake while resolving conflicts by rebasing. Let me fix it.

Airblader · 2021-12-16T07:54:08Z

flink-runtime-web/web-dashboard/src/app/share/common/dagre/svg-container.component.ts

+        this.zoom = k;
+        this.zoomEvent.emit(k);
+        this.containerTransform = transform;
+        this.transformEvent.emit(transform);


Why do we need the additional emit here?

same as above

yangjunhan · 2021-12-17T02:43:53Z

I think this will confuse the majority of developers as most do not work on the UI. Can we do something to restore the old behavior of not producing output unless the commit contains changes to the UI?

@Airblader There is such output because "flink-runtime-web" is introducing husky + lint-staged for linting which essentially uses git hook through pre-commit, and the pre-commit is at the repo level.

One way I can think of to restore the older behavior is triggering npm script lint-staged only when git diff --cached --quiet -- "flink-runtime-web/web-dashboard/*" (maybe written in an independent shell script). Could you think of any other better way to achieve so?

Airblader · 2022-01-10T11:45:03Z

@yangjunhan I apologize for the late response; I was on vacation over the holidays.

The changes overall look fine to me, except for the open point around the linting output. I think the solution you proposed sounds good, at least I can't think of anything better either.

…anged

yangjunhan · 2022-01-14T03:54:59Z

@yangjunhan I apologize for the late response; I was on vacation over the holidays.

The changes overall look fine to me, except for the open point around the linting output. I think the solution you proposed sounds good, at least I can't think of anything better either.

@Airblader Hope you enjoyed your holidays. Since NG-ZORRO-antd recently released its new version, I am considering bumping Runtime-Web directly to Angular v13 and NG-ZORRO-antd v13 in this PR, if that is ok with you and @MartijnVisser .

MartijnVisser · 2022-01-14T04:52:34Z

@yangjunhan I'm not sure if it's better to make it a separate PR (since we also have https://issues.apache.org/jira/browse/FLINK-24768 open for this) or if we should do it in a separate commit. Let's see what @Airblader says on that. It would be great do have this update in place!

Airblader · 2022-01-14T07:08:53Z

I would prefer bumping this in a separate PR using the JIRA issue @MartijnVisser pointed to. I'm happy to review and merge that one as well, of course (no vacation in between this time).

yangjunhan · 2022-01-17T01:48:45Z

I would prefer bumping this in a separate PR using the JIRA issue @MartijnVisser pointed to. I'm happy to review and merge that one as well, of course (no vacation in between this time).

@Airblader No problems! I can do it in a separate PR. Meanwhile, I fixed the linting output problem as we discussed.

Airblader

Thanks! I'm rebasing this and letting it run through the CI one more time before merging.

resolves apache#17711

rmetzger added the component=<none> label Nov 8, 2021

Airblader reviewed Nov 8, 2021

View reviewed changes

flink-runtime-web/web-dashboard/package.json Outdated Show resolved Hide resolved

yangjunhan force-pushed the chore/runtime-web branch from 80fbcfa to edcadd9 Compare November 9, 2021 01:47

yangjunhan force-pushed the chore/runtime-web branch from edcadd9 to 15f87a5 Compare December 15, 2021 06:19

yangjunhan force-pushed the chore/runtime-web branch from 15f87a5 to d143a92 Compare December 15, 2021 09:01

Airblader requested changes Dec 16, 2021

View reviewed changes

yangjunhan force-pushed the chore/runtime-web branch from d143a92 to b4e9783 Compare December 17, 2021 02:46

yangjunhan requested a review from Airblader December 20, 2021 01:37

yangjunhan added 2 commits January 14, 2022 09:51

[FLINK-24737][runtime-web] Update outdated web dependencies

aae8564

[FLINK-24737][runtime-web] Silence lint-staged if runtime-web is unch…

01cc7a5

…anged

yangjunhan force-pushed the chore/runtime-web branch from b4e9783 to 01cc7a5 Compare January 14, 2022 03:51

Airblader approved these changes Jan 17, 2022

View reviewed changes

Airblader closed this in 3722d3f Jan 17, 2022

niklassemmler pushed a commit to niklassemmler/flink that referenced this pull request Feb 3, 2022

[FLINK-24737][runtime-web] Update outdated web dependencies

29a5553

resolves apache#17711

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FLINK-24737][runtime-web] Update outdated web dependencies #17711

[FLINK-24737][runtime-web] Update outdated web dependencies #17711

yangjunhan commented Nov 8, 2021

flinkbot commented Nov 8, 2021 •

edited

Loading

flinkbot commented Nov 8, 2021

MartijnVisser commented Dec 10, 2021

yangjunhan commented Dec 15, 2021

MartijnVisser commented Dec 15, 2021

yangjunhan commented Dec 15, 2021

MartijnVisser commented Dec 15, 2021

MartijnVisser commented Dec 15, 2021

Airblader left a comment

Airblader Dec 16, 2021

Airblader Dec 16, 2021

yangjunhan Dec 16, 2021

Airblader Dec 16, 2021

yangjunhan Dec 16, 2021

yangjunhan commented Dec 17, 2021

Airblader commented Jan 10, 2022

yangjunhan commented Jan 14, 2022

MartijnVisser commented Jan 14, 2022

Airblader commented Jan 14, 2022 •

edited

Loading

yangjunhan commented Jan 17, 2022

Airblader left a comment •

edited

Loading

[FLINK-24737][runtime-web] Update outdated web dependencies #17711

[FLINK-24737][runtime-web] Update outdated web dependencies #17711

Conversation

yangjunhan commented Nov 8, 2021

What is the purpose of the change

Brief change log

Verifying this change

Does this pull request potentially affect one of the following parts:

Documentation

flinkbot commented Nov 8, 2021 • edited Loading

CI report:

flinkbot commented Nov 8, 2021

Automated Checks

Review Progress

MartijnVisser commented Dec 10, 2021

yangjunhan commented Dec 15, 2021

MartijnVisser commented Dec 15, 2021

yangjunhan commented Dec 15, 2021

MartijnVisser commented Dec 15, 2021

MartijnVisser commented Dec 15, 2021

Airblader left a comment

Choose a reason for hiding this comment

Airblader Dec 16, 2021

Choose a reason for hiding this comment

Airblader Dec 16, 2021

Choose a reason for hiding this comment

yangjunhan Dec 16, 2021

Choose a reason for hiding this comment

Airblader Dec 16, 2021

Choose a reason for hiding this comment

yangjunhan Dec 16, 2021

Choose a reason for hiding this comment

yangjunhan commented Dec 17, 2021

Airblader commented Jan 10, 2022

yangjunhan commented Jan 14, 2022

MartijnVisser commented Jan 14, 2022

Airblader commented Jan 14, 2022 • edited Loading

yangjunhan commented Jan 17, 2022

Airblader left a comment • edited Loading

Choose a reason for hiding this comment

flinkbot commented Nov 8, 2021 •

edited

Loading

Airblader commented Jan 14, 2022 •

edited

Loading

Airblader left a comment •

edited

Loading