rfellows commented on code in PR #8737:
URL: https://github.com/apache/nifi/pull/8737#discussion_r1589607420


##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/src/app/pages/flow-designer/ui/canvas/items/flow/import-from-registry/import-from-registry.component.ts:
##########
@@ -191,23 +195,30 @@ export class ImportFromRegistry implements OnInit {
 
         this.getBuckets(registryId)
             .pipe(take(1))
-            .subscribe((buckets: BucketEntity[]) => {
-                if (buckets.length > 0) {
-                    buckets.forEach((entity: BucketEntity) => {
-                        if (entity.permissions.canRead) {
-                            this.bucketOptions.push({
-                                text: entity.bucket.name,
-                                value: entity.id,
-                                description: entity.bucket.description
-                            });
-                        }
-                    });
+            .subscribe({
+                next: (buckets: BucketEntity[]) => {
+                    if (buckets.length > 0) {
+                        buckets.forEach((entity: BucketEntity) => {
+                            if (entity.permissions.canRead) {
+                                this.bucketOptions.push({
+                                    text: entity.bucket.name,
+                                    value: entity.id,
+                                    description: entity.bucket.description
+                                });
+                            }
+                        });
 
-                    const bucketId = this.bucketOptions[0].value;
-                    if (bucketId) {
-                        
this.importFromRegistryForm.get('bucket')?.setValue(bucketId);
-                        this.loadFlows(registryId, bucketId);
+                        const bucketId = this.bucketOptions[0].value;
+                        if (bucketId) {
+                            
this.importFromRegistryForm.get('bucket')?.setValue(bucketId);
+                            this.loadFlows(registryId, bucketId);
+                        }
                     }
+                },
+                error: (errorResponse: HttpErrorResponse) => {
+                    this.store.dispatch(
+                        FlowActions.flowBannerError({ error: 
this.errorHelper.getErrorString(errorResponse) })
+                    );

Review Comment:
   We've been pretty consistent in handling errors in the effects classes 
rather than in the component classes. Was there a particular reason you chose 
to handle the errors in here?
   
   I assumed the error handling would have been done in the flow.effects in a 
tap like so: 
   ```
                           dialogReference.componentInstance.getBuckets = (
                               registryId: string
                           ): Observable<BucketEntity[]> => {
                               return 
this.registryService.getBuckets(registryId).pipe(
                                   take(1),
                                   map((response) => response.buckets),
                                   tap({
                                       error: (errorResponse: 
HttpErrorResponse) => {
                                           this.store.dispatch(
                                               FlowActions.flowBannerError({ 
error: this.errorHelper.getErrorString(errorResponse) })
                                           );
                                       }
                                   })
                               );
                           };
   ```
   



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

Reply via email to