[NIFI-13053] Cluster page #8685

rfellows · 2024-04-22T18:02:17Z

New Cluster page accessible from the main menu
Listings for:
- Nodes
- System
- JVM
- FlowFile Storage
- Content Storage
- Provenance Storage
- Versions

All table here extend a base table ClusterTable and leverage the same filter component. The "Storage" pages all share the same same table implementation.

mcgilman · 2024-04-22T18:04:14Z

Will review...

mcgilman

Nice work @rfellows! I've left a few comments below. Most are minor but did notice one bigger issue. In the existing NiFi UI, the tabs that are populated through system diagnostics are hidden when the user lacks those permissions.

mcgilman · 2024-04-23T15:50:16Z

...-web/nifi-web-frontend/src/main/nifi/src/app/pages/cluster/feature/cluster-routing.module.ts

+const routes: Routes = [
+    {
+        path: '',
+        component: Cluster,


Based on the changes to navigation.component.html we should be using the authorizationGuard to ensure the user has:

user.controllerPermissions.canRead

mcgilman · 2024-04-23T15:52:22Z

...rontend/src/main/nifi/src/app/pages/cluster/state/cluster-listing/cluster-listing.effects.ts

+                from(this.clusterService.disconnectNode(request.nodeId)).pipe(
+                    map((entity) => {
+                        this.dialog.closeAll();
+                        return ClusterListingActions.updateNodeSuccess({ response: entity });
+                    }),
+                    catchError((errorResponse: HttpErrorResponse) => {
+                        this.dialog.closeAll();
+                        return of(ClusterListingActions.clusterNodeSnackbarError({ error: errorResponse.error }));
+                    })
+                )


The this.dialog.closeAll() in this effect is unnecessary. The YesNoDialog does not stay open when the user presses Yes or No.

mcgilman · 2024-04-23T15:52:29Z

...rontend/src/main/nifi/src/app/pages/cluster/state/cluster-listing/cluster-listing.effects.ts

+                from(this.clusterService.connectNode(request.nodeId)).pipe(
+                    map((entity) => {
+                        this.dialog.closeAll();
+                        return ClusterListingActions.updateNodeSuccess({ response: entity });
+                    }),
+                    catchError((errorResponse: HttpErrorResponse) => {
+                        this.dialog.closeAll();
+                        return of(ClusterListingActions.clusterNodeSnackbarError({ error: errorResponse.error }));
+                    })
+                )


The this.dialog.closeAll() in this effect is unnecessary. The YesNoDialog does not stay open when the user presses Yes or No.

mcgilman · 2024-04-23T15:52:42Z

...rontend/src/main/nifi/src/app/pages/cluster/state/cluster-listing/cluster-listing.effects.ts

+                from(this.clusterService.offloadNode(request.nodeId)).pipe(
+                    map((entity) => {
+                        this.dialog.closeAll();
+                        return ClusterListingActions.updateNodeSuccess({ response: entity });
+                    }),
+                    catchError((errorResponse: HttpErrorResponse) => {
+                        this.dialog.closeAll();
+                        return of(ClusterListingActions.clusterNodeSnackbarError({ error: errorResponse.error }));
+                    })
+                )


The this.dialog.closeAll() in this effect is unnecessary. The YesNoDialog does not stay open when the user presses Yes or No.

mcgilman · 2024-04-23T15:52:59Z

...rontend/src/main/nifi/src/app/pages/cluster/state/cluster-listing/cluster-listing.effects.ts

+                from(this.clusterService.removeNode(request.nodeId)).pipe(
+                    map(() => {
+                        this.dialog.closeAll();
+                        return ClusterListingActions.removeNodeSuccess({ response: request });
+                    }),
+                    catchError((errorResponse: HttpErrorResponse) => {
+                        this.dialog.closeAll();
+                        return of(ClusterListingActions.clusterNodeSnackbarError({ error: errorResponse.error }));
+                    })
+                )


The this.dialog.closeAll() in this effect is unnecessary. The YesNoDialog does not stay open when the user presses Yes or No.

mcgilman · 2024-04-23T16:07:46Z

...ntend/src/main/nifi/src/app/pages/cluster/ui/common/cluster-table/cluster-table.component.ts

+        return this._initialSortDirection;
+    }
+
+    @Input({}) set components(components: T[]) {


Is this {} empty object needed here?

mcgilman · 2024-04-23T17:54:23Z

.../app/pages/cluster/ui/cluster-jvm-listing/cluster-jvm-table/cluster-jvm-table.component.html

+                            <!-- Node Address -->
+                            <ng-container matColumnDef="address">
+                                <th mat-header-cell *matHeaderCellDef mat-sort-header title="Node Address">
+                                    <div class="flex-1 overflow-ellipsis overflow-hidden whitespace-nowrap">


I don't think that flex-1 isn needed in our column headers. This would apply to all headers in all tables in this PR.

mcgilman · 2024-04-23T18:34:56Z

...ifi/src/app/pages/cluster/ui/common/cluster-table-filter/cluster-table-filter.component.scss

+.cluster-table-filter-container {
+}


Can this be removed?

mcgilman · 2024-04-23T18:36:16Z

...ter/ui/cluster-version-listing/cluster-version-table/cluster-version-table.component.spec.ts

+import { ClusterVersionTable } from './cluster-version-table.component';
+import { NoopAnimationsModule } from '@angular/platform-browser/animations';
+
+describe('ClusterVersionTableComponent', () => {


Suggested change

describe('ClusterVersionTableComponent', () => {

describe('ClusterVersionTable', () => {

mcgilman · 2024-04-23T18:47:31Z

.../nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/cluster/feature/cluster.component.ts

+        { label: 'System', link: 'system' },
+        { label: 'JVM', link: 'jvm' },
+        { label: 'FlowFile Storage', link: 'flowfile-storage' },
+        { label: 'Content Storage', link: 'content-storage' },
+        { label: 'Provenance Storage', link: 'provenance-storage' },
+        { label: 'Versions', link: 'versions' }


The presence of these tabs should be conditional on the users permissions to System Diagnostics.

mcgilman

Thanks for the updates @rfellows! Will merge once CI completes.

* Node listing

…the user has the proper permission.

…state when user loses permission to it while on the cluster page.

* [NIFI-13053] - Cluster Node Table/Page * Node listing * System Listing * Jvm Listing * FlowFile Storage listing * Content and Provenance Repo Storage listings * Version listing * review feedback * only attempt to load system diagnostic info for cluster node view if the user has the proper permission. * Move Cluster Summary loading/polling to the navigation component. * restore user state resetting when users component is destroyed. * reset state on cluster component destroy and reset system diagnostic state when user loses permission to it while on the cluster page. This closes apache#8685

rfellows added the new ui Pull requests for work relating to the new user interface being developed. label Apr 22, 2024

mcgilman self-requested a review April 22, 2024 18:04

mcgilman reviewed Apr 23, 2024

View reviewed changes

rfellows force-pushed the NIFI-13053-nodeTable branch from 0cbc3a7 to 6a6fe89 Compare April 25, 2024 13:01

mcgilman approved these changes Apr 25, 2024

View reviewed changes

rfellows added 11 commits April 25, 2024 12:26

[NIFI-13053] - Cluster Node Table/Page

f929a5d

* Node listing

System Listing

3471cb8

Jvm Listing

1a566b4

FlowFile Storage listing

2ef41bc

Content and Provenance Repo Storage listings

c0a1dcf

Version listing

fbd1f03

review feedback

00f09ab

only attempt to load system diagnostic info for cluster node view if …

48f4718

…the user has the proper permission.

Move Cluster Summary loading/polling to the navigation component.

39895e3

restore user state resetting when users component is destroyed.

feea740

reset state on cluster component destroy and reset system diagnostic …

6c00aaf

…state when user loses permission to it while on the cluster page.

rfellows force-pushed the NIFI-13053-nodeTable branch from 58c1bf2 to 6c00aaf Compare April 25, 2024 16:27

mcgilman merged commit 0311bff into apache:main Apr 25, 2024
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[NIFI-13053] Cluster page #8685

[NIFI-13053] Cluster page #8685

rfellows commented Apr 22, 2024

mcgilman commented Apr 22, 2024

mcgilman left a comment

mcgilman Apr 23, 2024

mcgilman Apr 23, 2024

mcgilman Apr 23, 2024

mcgilman Apr 23, 2024

mcgilman Apr 23, 2024

mcgilman Apr 23, 2024

mcgilman Apr 23, 2024

mcgilman Apr 23, 2024

mcgilman Apr 23, 2024

mcgilman Apr 23, 2024

mcgilman left a comment

	describe('ClusterVersionTableComponent', () => {
	describe('ClusterVersionTable', () => {

		.cluster-table-filter-container {
		}

[NIFI-13053] Cluster page #8685

[NIFI-13053] Cluster page #8685

Conversation

rfellows commented Apr 22, 2024

mcgilman commented Apr 22, 2024

mcgilman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcgilman left a comment

Choose a reason for hiding this comment