[GitHub] [metron] tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests
tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts
UI: Indicating loading and preventing parallel requests
URL: https://github.com/apache/metron/pull/1514#discussion_r327094011
##
File path: metron-interface/metron-alerts/src/app/utils/httpUtil.ts
##
@@ -33,9 +33,12 @@ export class HttpUtil {
return body || {};
}
+ /**
+ * @depricated Turning all errors to 404 and hiding actual errors from the
consumers
Review comment:
Fixed. Thanks
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
With regards,
Apache Git Services
[GitHub] [metron] tiborm opened a new pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests
tiborm opened a new pull request #1514: METRON-2190: [UI] Alerts UI: Indicating
loading and preventing parallel requests
URL: https://github.com/apache/metron/pull/1514
## Contributor Comments
As a part of this pull request I introduce the concepts of blocking and non
blocking search queries for Alerts UI. With this feature UI can give feedback
to our users about the ongoing query process. Also, can prevent sending
multiple requests in a row, before getting response or failure.
As a prerequisite of this I had to separate blocking and non-blocking query
operations.
Blocking query operations (triggered by user)
- querying/searching by search button
- page size change
- sorting
- grouping
- show/hide dismissed or resolved items
Non-blocking query operation is basically
- automatic polling (triggered by timer)
## Congestion handling
### Search triggered by the User
Congestion is no longer possible with queries triggered by the user because
of the modal loading indicator. This indicator prevents any interaction till
the request resolves with a result or a failure.
### Automatic polling
In case the response time is longer than the polling interval user, able to
overload the back-end. Simply by automatically sending a new request before the
previous one get processed on the next interval. (If the response time is 30
sec and the interval is 10, this means 3 parallel request. Each one of them
increasing the processing time further.)
To protect our services from auto polling overload UI skipping every polling
cycle till a pending request exists. In this scenario the UI shows a warning to
the User about the congestion and ask to increase the polling interval or
optimize the query filter.
## Small improvements on auto polling
- auto polling sends an initial request on start
- auto polling state persisted
- on/of state
- interval restored
## Other related changes
- surrounding code had to be deeply refactored, to decouple search, auto
poll and row config
- auto polling logic moved to a single service (previously it was shared
across alerts-list.component, search.service and query-builder.service)
- conflicts with manual filtering feature
- manual query/filter string and the belonging logic had to be moved
from alerts-list.component to query-builder
- I restored the "single source of query string", no matter it is made
by the builder logic or manually by the user
- with this auto polling able to pick up the latest query in every
interval from one single source
## Known issues currently in the master
There is a few known issue in the current code base, what might interfere
with this changeset while your are testing. Please keep in mind, these are
already reproducible on the master branch and not introduced by this. To keep
this PR in a reasonable size, I haven't try to fix them as a part of current
changeset. However, as a positive impact of all of the refactoring work we did
we'll be able to fix them more easily on the top of this PR.
[[UI] Switching back from manual filtering mode eliminates time range filter
from query](https://issues.apache.org/jira/browse/METRON-2258)
[[UI] Hide Resolved and Hide Dismissed toggles not works when filtering is
in manual mode](https://issues.apache.org/jira/browse/METRON-2259)
[[UI] Performance: Switching manual filtering on and off multiple times
leads slow typing](https://issues.apache.org/jira/browse/METRON-2260)
I'm planning to target these in followup PRs.
## Testing
### Scenario 1 (manual querying)
- Search queries initiated by the following interactions
- querying/searching by search button
- page size change
- sorting
- grouping
- show/hide dismissed or resolved items
- should be indicated by a modal loader
- The User has to be prevented from any further interaction on the UI while
the operation is in progress.
Scenario 1.1 (error handling)
- An error dialogue should show up on the UI in case of internal server
error.
### Scenario 2 (auto polling)
- User should be able to start/stop auto polling by the play/pause icon
below the searh button
Scenario 2.1 (error handling)
- An error dialogue should show up on the UI in case of internal server
error.
Scenario 2.2 (congestion handling)
- When the user set up a short polling interval
- And the response time is slower then polling interval
- A non modal warning should show up for the user
- All further polling request should be skipped to avoid overload until a
response, timeout or error occur
### Scenario 3 (polling suppression)
- When auto polling is turned on
- The following operations should temporarily suppress polling
- selecting alerts by checkbox
- showing details pane
- showing save search pane
[GitHub] [metron] tiborm closed pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests
tiborm closed pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests URL: https://github.com/apache/metron/pull/1514 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 With regards, Apache Git Services
[GitHub] [metron] tiborm commented on issue #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests
tiborm commented on issue #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests URL: https://github.com/apache/metron/pull/1514#issuecomment-534076602 closing and opening to trigger new build 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 With regards, Apache Git Services
[GitHub] [metron] ruffle1986 commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests
ruffle1986 commented on a change in pull request #1514: METRON-2190: [UI]
Alerts UI: Indicating loading and preventing parallel requests
URL: https://github.com/apache/metron/pull/1514#discussion_r327016495
##
File path: metron-interface/metron-alerts/src/app/utils/httpUtil.ts
##
@@ -33,9 +33,12 @@ export class HttpUtil {
return body || {};
}
+ /**
+ * @depricated Turning all errors to 404 and hiding actual errors from the
consumers
Review comment:
I think you have meant `@deprecated`
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
With regards,
Apache Git Services
[GitHub] [metron] tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests
tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts
UI: Indicating loading and preventing parallel requests
URL: https://github.com/apache/metron/pull/1514#discussion_r327021709
##
File path: metron-interface/metron-alerts/src/app/utils/httpUtil.ts
##
@@ -33,9 +33,12 @@ export class HttpUtil {
return body || {};
}
+ /**
+ * @depricated Turning all errors to 404 and hiding actual errors from the
consumers
+ * could limit how we can recover or react to errors.
+ * Use sessionExpiration instead and/or introduce new composable
error handlers.
+ */
public static handleError(res: HttpErrorResponse): Observable {
-// In a real world app, we might use a remote logging infrastructure
-// We'd also dig deeper into the error to get a better message
Review comment:
If we once accepted to have comments like this in the code using // FIXME is
also acceptable I think. :)
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
With regards,
Apache Git Services
[GitHub] [metron] tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests
tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts
UI: Indicating loading and preventing parallel requests
URL: https://github.com/apache/metron/pull/1514#discussion_r327021709
##
File path: metron-interface/metron-alerts/src/app/utils/httpUtil.ts
##
@@ -33,9 +33,12 @@ export class HttpUtil {
return body || {};
}
+ /**
+ * @depricated Turning all errors to 404 and hiding actual errors from the
consumers
+ * could limit how we can recover or react to errors.
+ * Use sessionExpiration instead and/or introduce new composable
error handlers.
+ */
public static handleError(res: HttpErrorResponse): Observable {
-// In a real world app, we might use a remote logging infrastructure
-// We'd also dig deeper into the error to get a better message
Review comment:
If we once accepted to have comments like this in the code using // FIXME is
also acceptable I think. :)
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
With regards,
Apache Git Services
[GitHub] [metron] tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests
tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts
UI: Indicating loading and preventing parallel requests
URL: https://github.com/apache/metron/pull/1514#discussion_r326994513
##
File path:
metron-interface/metron-alerts/src/app/alerts/alerts-list/alerts-list.component.spec.ts
##
@@ -28,53 +28,136 @@ import { SaveSearchService } from
'app/service/save-search.service';
import { MetaAlertService } from 'app/service/meta-alert.service';
import { GlobalConfigService } from 'app/service/global-config.service';
import { DialogService } from 'app/service/dialog.service';
-import { SearchRequest } from 'app/model/search-request';
-import { Observable, of, Subject } from 'rxjs';
-import { Filter } from 'app/model/filter';
-import { QueryBuilder } from './query-builder';
import { TIMESTAMP_FIELD_NAME } from 'app/utils/constants';
-import { SearchResponse } from 'app/model/search-response';
import { By } from '@angular/platform-browser';
+import { Observable, Subject, of, throwError } from 'rxjs';
+import { Filter } from 'app/model/filter';
+import { QueryBuilder, FilteringMode } from './query-builder';
+import { SearchResponse } from 'app/model/search-response';
+import { AutoPollingService } from './auto-polling/auto-polling.service';
+import { Router } from '@angular/router';
+import { Alert } from 'app/model/alert';
+import { AlertSource } from 'app/model/alert-source';
+import { SearchRequest } from 'app/model/search-request';
+import { query } from '@angular/core/src/render3';
+import { RestError } from 'app/model/rest-error';
+import { DialogType } from 'app/shared/metron-dialog/metron-dialog.component';
+
+@Component({
+ selector: 'app-auto-polling',
+ template: '',
+})
+class MockAutoPollingComponent {}
+
+@Component({
+ selector: 'app-configure-rows',
+ template: '',
+})
+class MockConfigureRowsComponent {
+ @Input() refreshInterval = 0;
+ @Input() srcElement = {};
+ @Input() pageSize = 0;
+}
+
+@Component({
+ selector: 'app-modal-loading-indicator',
+ template: '',
+})
+class MockModalLoadingIndicatorComponent {
+ @Input() show = false;
+}
+
+@Component({
+ selector: 'app-time-range',
+ template: '',
+})
+class MockTimeRangeComponent {
+ @Input() disabled = false;
+ @Input() selectedTimeRange = {};
+}
+
+@Directive({
+ selector: '[appAceEditor]',
+})
+class MockAceEditorDirective {
+ @Input() text = '';
+}
+
+@Component({
+ selector: 'app-alert-filters',
+ template: '',
+})
+class MockAlertFilterComponent {
+ @Input() facets = [];
+}
+
+@Component({
+ selector: 'app-group-by',
+ template: '',
+})
+class MockGroupByComponent {
+ @Input() facets = [];
+}
+
+@Component({
+ selector: 'app-table-view',
+ template: '',
+})
+class MockTableViewComponent {
+ @Input() alerts = [];
+ @Input() pagination = {};
+ @Input() alertsColumnsToDisplay = [];
+ @Input() selectedAlerts = [];
+}
+
+@Component({
+ selector: 'app-tree-view',
+ template: '',
+})
+class MockTreeViewComponent {
+ @Input() alerts = [];
+ @Input() pagination = {};
+ @Input() alertsColumnsToDisplay = [];
+ @Input() selectedAlerts = [];
+ @Input() globalConfig = {};
+ @Input() query = '';
+ @Input() groups = [];
+}
+
Review comment:
With mocking out all the visual sub components instead of importing the
actual ones the test become faster.
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
With regards,
Apache Git Services
[GitHub] [metron] tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests
tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts
UI: Indicating loading and preventing parallel requests
URL: https://github.com/apache/metron/pull/1514#discussion_r327008834
##
File path:
metron-interface/metron-alerts/src/app/alerts/alerts-list/alerts-list.component.ts
##
@@ -549,24 +522,52 @@ export class AlertsListComponent implements OnInit,
OnDestroy {
this.cdRef.detectChanges();
}
- toggleQueryBuilder() {
+ getStaleDataWarning() {
+if (this.autoPollingSvc.getIsPollingActive()) {
+ return ` Data is in a
stale state!
+Click to update your
view based
+on your current filter and time-range configuration!`;
+} else {
+ return ` Data is in a
stale state!
+Automatic refresh is turned on. Your filter and/or time-range changes
will apply automatically on next refresh.`;
+}
+ }
+
+ getPollingCongestionWarning() {
+return ` Refresh interval
is shorter than the response time.
+ Please increase the refresh interval in the menu above,
+ or try to simplify your query filter.`;
+ }
+
+ private updatePollingInterval(refreshInterval: number): void {
+this.autoPollingSvc.setInterval(refreshInterval);
+ }
+
+ private restoreAutoPollingState() {
+if (this.autoPollingSvc.getIsPollingActive()) {
+ this.autoPollingSvc.setSuppression(false);
+}
+ }
+
+ isQueryBuilderModeManual() {
+return this.queryBuilder.getFilteringMode() === FilteringMode.MANUAL;
+ }
+
+ toggleQueryBuilderMode() {
+// FIXME setting timerange on toggle feels like a hack
this.setSelectedTimeRange([this.selectedTimeRange]);
-if (!this.hideQueryBuilder) {
- this.hideQueryBuilder = true;
- this.manualQuery.nativeElement.value = this.queryBuilder.query;
+if (this.queryBuilder.getFilteringMode() === FilteringMode.BUILDER) {
+ this.queryBuilder.setFilteringMode(FilteringMode.MANUAL);
} else {
- this.hideQueryBuilder = false;
+ this.queryBuilder.setFilteringMode(FilteringMode.BUILDER);
+ // FIXME: this could lead to a large blocking load depending on the
response time
this.queryBuilder.clearSearch();
this.search();
}
}
queryForTreeView() {
-if (!this.hideQueryBuilder) {
- return this.queryBuilder.generateSelect();
-} else {
- return this.manualQuery.nativeElement.value;
-}
Review comment:
This is a good example when we having two separated source of the same
value. Merged into one. The logic encapsulated in query builder and the
difference between manual and built query in no longer exposed.
(Query builder is never hidden or turned off only the filter bar changes
depending on this flag.)
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
With regards,
Apache Git Services
[GitHub] [metron] tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests
tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts
UI: Indicating loading and preventing parallel requests
URL: https://github.com/apache/metron/pull/1514#discussion_r326996126
##
File path:
metron-interface/metron-alerts/src/app/alerts/alerts-list/alerts-list.component.spec.ts
##
@@ -28,53 +28,136 @@ import { SaveSearchService } from
'app/service/save-search.service';
import { MetaAlertService } from 'app/service/meta-alert.service';
import { GlobalConfigService } from 'app/service/global-config.service';
import { DialogService } from 'app/service/dialog.service';
-import { SearchRequest } from 'app/model/search-request';
-import { Observable, of, Subject } from 'rxjs';
-import { Filter } from 'app/model/filter';
-import { QueryBuilder } from './query-builder';
import { TIMESTAMP_FIELD_NAME } from 'app/utils/constants';
-import { SearchResponse } from 'app/model/search-response';
import { By } from '@angular/platform-browser';
+import { Observable, Subject, of, throwError } from 'rxjs';
+import { Filter } from 'app/model/filter';
+import { QueryBuilder, FilteringMode } from './query-builder';
+import { SearchResponse } from 'app/model/search-response';
+import { AutoPollingService } from './auto-polling/auto-polling.service';
+import { Router } from '@angular/router';
+import { Alert } from 'app/model/alert';
+import { AlertSource } from 'app/model/alert-source';
+import { SearchRequest } from 'app/model/search-request';
+import { query } from '@angular/core/src/render3';
+import { RestError } from 'app/model/rest-error';
+import { DialogType } from 'app/shared/metron-dialog/metron-dialog.component';
+
+@Component({
+ selector: 'app-auto-polling',
+ template: '',
+})
+class MockAutoPollingComponent {}
+
+@Component({
+ selector: 'app-configure-rows',
+ template: '',
+})
+class MockConfigureRowsComponent {
+ @Input() refreshInterval = 0;
+ @Input() srcElement = {};
+ @Input() pageSize = 0;
+}
+
+@Component({
+ selector: 'app-modal-loading-indicator',
+ template: '',
+})
+class MockModalLoadingIndicatorComponent {
+ @Input() show = false;
+}
+
+@Component({
+ selector: 'app-time-range',
+ template: '',
+})
+class MockTimeRangeComponent {
+ @Input() disabled = false;
+ @Input() selectedTimeRange = {};
+}
+
+@Directive({
+ selector: '[appAceEditor]',
+})
+class MockAceEditorDirective {
+ @Input() text = '';
+}
+
+@Component({
+ selector: 'app-alert-filters',
+ template: '',
+})
+class MockAlertFilterComponent {
+ @Input() facets = [];
+}
+
+@Component({
+ selector: 'app-group-by',
+ template: '',
+})
+class MockGroupByComponent {
+ @Input() facets = [];
+}
+
+@Component({
+ selector: 'app-table-view',
+ template: '',
+})
+class MockTableViewComponent {
+ @Input() alerts = [];
+ @Input() pagination = {};
+ @Input() alertsColumnsToDisplay = [];
+ @Input() selectedAlerts = [];
+}
+
+@Component({
+ selector: 'app-tree-view',
+ template: '',
+})
+class MockTreeViewComponent {
+ @Input() alerts = [];
+ @Input() pagination = {};
+ @Input() alertsColumnsToDisplay = [];
+ @Input() selectedAlerts = [];
+ @Input() globalConfig = {};
+ @Input() query = '';
+ @Input() groups = [];
+}
+
describe('AlertsListComponent', () => {
let component: AlertsListComponent;
let fixture: ComponentFixture;
- let searchServiceStub = {
-search() { return of({
- total: 0,
- groupedBy: '',
- results: [],
- facetCounts: [],
- groups: []
-}) },
-pollSearch() { return of({}) }
- }
- let queryBuilderStub = {
-addOrUpdateFilter() { return {} },
-clearSearch() { return {} },
-generateSelect() { return '*' },
-isTimeStampFieldPresent() { return {} },
-filters: [{}],
-searchRequest: {
- from: 0
-}
- }
let queryBuilder: QueryBuilder;
let searchService: SearchService;
beforeEach(async(() => {
+
+const searchResponseFake = new SearchResponse();
+searchResponseFake.facetCounts = {};
+
TestBed.configureTestingModule({
- schemas: [ NO_ERRORS_SCHEMA ],
Review comment:
NO_ERROR_SCHEMA is no longer needed bc I defined mocks for all sub component.
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
With regards,
Apache Git Services
[GitHub] [metron] tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests
tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts
UI: Indicating loading and preventing parallel requests
URL: https://github.com/apache/metron/pull/1514#discussion_r327018486
##
File path:
metron-interface/metron-alerts/src/app/alerts/configure-rows/show-hide/show-hide-alert-entries.component.spec.ts
##
@@ -104,29 +104,26 @@ describe('ShowHideAlertEntriesComponent', () => {
expect(component.onVisibilityChanged).toHaveBeenCalledWith('DISMISS',
false);
});
- it('should trigger changed event on any toggle changes', () => {
+ it('should trigger changed event on any toggle changes and propagate state',
() => {
+const serviceSpy = TestBed.get(ShowHideService);
spyOn(component.changed, 'emit');
fixture.detectChanges();
-
fixture.debugElement.query(By.css('[data-qe-id="hideDismissedAlertsToggle"]
input')).nativeElement.click();
-fixture.detectChanges();
-
-expect((component.changed.emit as Spy).calls.argsFor(0)[0]).toEqual(new
ShowHideChanged('DISMISS', true));
-
-fixture.debugElement.query(By.css('[data-qe-id="hideResolvedAlertsToggle"]
input')).nativeElement.click();
-fixture.detectChanges();
-
-expect((component.changed.emit as Spy).calls.argsFor(1)[0]).toEqual(new
ShowHideChanged('RESOLVE', true));
Review comment:
The content of the event is changed to what you see in line 118 and 126.
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
With regards,
Apache Git Services
[GitHub] [metron] tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests
tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts
UI: Indicating loading and preventing parallel requests
URL: https://github.com/apache/metron/pull/1514#discussion_r327005220
##
File path:
metron-interface/metron-alerts/src/app/alerts/alerts-list/alerts-list.component.ts
##
@@ -460,69 +472,30 @@ export class AlertsListComponent implements OnInit,
OnDestroy {
}
showConfigureTable() {
-this.saveRefreshState();
+this.autoPollingSvc.setSuppression(true);
this.router.navigateByUrl('/alerts-list(dialog:configure-table)');
}
showDetails(alert: Alert) {
this.selectedAlerts = [];
this.selectedAlerts = [alert];
-this.saveRefreshState();
+this.autoPollingSvc.setSuppression(true);
let sourceType = alert.source[this.globalConfig['source.type.field']];
let url = '/alerts-list(dialog:details/' + sourceType + '/' +
alert.source.guid + '/' + alert.index + ')';
this.router.navigateByUrl(url);
}
- saveRefreshState() {
-this.lastIsRefreshPausedValue = this.isRefreshPaused;
-this.tryStopPolling();
- }
-
- pause() {
-this.isRefreshPaused = true;
-this.tryStopPolling();
- }
-
- resume() {
-this.isRefreshPaused = false;
-this.tryStartPolling();
- }
-
showSavedSearches() {
-this.saveRefreshState();
+this.autoPollingSvc.setSuppression(true);
this.router.navigateByUrl('/alerts-list(dialog:saved-searches)');
}
showSaveSearch() {
-this.saveRefreshState();
+this.autoPollingSvc.setSuppression(true);
this.saveSearchService.setCurrentQueryBuilderAndTableColumns(this.queryBuilder,
this.alertsColumns);
this.router.navigateByUrl('/alerts-list(dialog:save-search)');
}
- tryStartPolling(manualSearch?: SearchRequest) {
-if (!this.isRefreshPaused && !manualSearch) {
- this.tryStopPolling();
- this.refreshTimer =
this.searchService.pollSearch(this.queryBuilder.searchRequest).subscribe(results
=> {
-this.setData(results);
- });
-} else if (!this.isRefreshPaused && manualSearch) {
- this.tryStopPolling();
- this.refreshTimer =
this.searchService.pollSearch(manualSearch).subscribe(results => {
-this.setData(results);
- });
-}
- }
-
- tryStopPolling() {
-if (this.refreshTimer && !this.refreshTimer.closed) {
- this.refreshTimer.unsubscribe();
-}
- }
-
- updateConfigRowsSettings() {
-this.searchService.interval = this.refreshInterval;
- }
-
Review comment:
Logic moved to auto-polling.service
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
With regards,
Apache Git Services
[GitHub] [metron] tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests
tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts
UI: Indicating loading and preventing parallel requests
URL: https://github.com/apache/metron/pull/1514#discussion_r326998004
##
File path:
metron-interface/metron-alerts/src/app/alerts/alerts-list/alerts-list.component.ts
##
@@ -99,17 +108,23 @@ export class AlertsListComponent implements OnInit,
OnDestroy {
private metaAlertsService: MetaAlertService,
private globalConfigService: GlobalConfigService,
private dialogService: DialogService,
+ private cdRef: ChangeDetectorRef,
public queryBuilder: QueryBuilder,
- private cdRef: ChangeDetectorRef) {
+ public autoPollingSvc: AutoPollingService) {
router.events.subscribe(event => {
if (event instanceof NavigationStart && event.url === '/alerts-list') {
this.selectedAlerts = [];
-this.restoreRefreshState();
+this.restoreAutoPollingState();
}
});
+
+autoPollingSvc.data.subscribe((result: SearchResponse) => {
+ this.setData(result);
+ this.staleDataState = false;
+})
Review comment:
I was separating auto polling from normal search to distinguish timer and
user triggered queries The output of auto polling is now an observable.
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
With regards,
Apache Git Services
[GitHub] [metron] tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests
tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts
UI: Indicating loading and preventing parallel requests
URL: https://github.com/apache/metron/pull/1514#discussion_r327007289
##
File path:
metron-interface/metron-alerts/src/app/alerts/alerts-list/alerts-list.component.ts
##
@@ -549,24 +522,52 @@ export class AlertsListComponent implements OnInit,
OnDestroy {
this.cdRef.detectChanges();
}
- toggleQueryBuilder() {
+ getStaleDataWarning() {
+if (this.autoPollingSvc.getIsPollingActive()) {
+ return ` Data is in a
stale state!
+Click to update your
view based
+on your current filter and time-range configuration!`;
+} else {
+ return ` Data is in a
stale state!
+Automatic refresh is turned on. Your filter and/or time-range changes
will apply automatically on next refresh.`;
+}
+ }
+
+ getPollingCongestionWarning() {
+return ` Refresh interval
is shorter than the response time.
+ Please increase the refresh interval in the menu above,
+ or try to simplify your query filter.`;
+ }
+
+ private updatePollingInterval(refreshInterval: number): void {
+this.autoPollingSvc.setInterval(refreshInterval);
+ }
+
+ private restoreAutoPollingState() {
+if (this.autoPollingSvc.getIsPollingActive()) {
+ this.autoPollingSvc.setSuppression(false);
+}
+ }
+
+ isQueryBuilderModeManual() {
+return this.queryBuilder.getFilteringMode() === FilteringMode.MANUAL;
+ }
+
+ toggleQueryBuilderMode() {
+// FIXME setting timerange on toggle feels like a hack
this.setSelectedTimeRange([this.selectedTimeRange]);
-if (!this.hideQueryBuilder) {
- this.hideQueryBuilder = true;
- this.manualQuery.nativeElement.value = this.queryBuilder.query;
+if (this.queryBuilder.getFilteringMode() === FilteringMode.BUILDER) {
+ this.queryBuilder.setFilteringMode(FilteringMode.MANUAL);
} else {
- this.hideQueryBuilder = false;
+ this.queryBuilder.setFilteringMode(FilteringMode.BUILDER);
+ // FIXME: this could lead to a large blocking load depending on the
response time
Review comment:
A separated jira was made out of this.
There are lot's of opinions about using FIXME. In here I intentionally kept
them bc alerts-list can't be cleared out/refactored in one iteration and
reminders about already identified issues can be handy later when we touching
this part of the code again.
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
With regards,
Apache Git Services
[GitHub] [metron] tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests
tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts
UI: Indicating loading and preventing parallel requests
URL: https://github.com/apache/metron/pull/1514#discussion_r327004638
##
File path:
metron-interface/metron-alerts/src/app/alerts/alerts-list/alerts-list.component.ts
##
@@ -311,16 +344,12 @@ export class AlertsListComponent implements OnInit,
OnDestroy {
prepareColumnData(configuredColumns: ColumnMetadata[], defaultColumns:
ColumnMetadata[]) {
this.alertsColumns = (configuredColumns && configuredColumns.length > 0) ?
configuredColumns : defaultColumns;
-this.queryBuilder.setFields(this.getColumnNamesForQuery());
this.calcColumnsToDisplay();
}
prepareData(tableMetaData: TableMetadata, defaultColumns: ColumnMetadata[]) {
-this.tableMetaData = tableMetaData;
-this.refreshInterval = this.tableMetaData.refreshInterval;
-
-this.updateConfigRowsSettings();
Review comment:
Logic moved to auto-polling.service
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
With regards,
Apache Git Services
[GitHub] [metron] tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests
tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts
UI: Indicating loading and preventing parallel requests
URL: https://github.com/apache/metron/pull/1514#discussion_r327004408
##
File path:
metron-interface/metron-alerts/src/app/alerts/alerts-list/alerts-list.component.ts
##
@@ -373,11 +402,6 @@ export class AlertsListComponent implements OnInit,
OnDestroy {
}
}
- restoreRefreshState() {
-this.isRefreshPaused = this.lastIsRefreshPausedValue;
-this.tryStartPolling();
- }
Review comment:
Logic moved to auto-polling.service
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
With regards,
Apache Git Services
[GitHub] [metron] tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests
tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts
UI: Indicating loading and preventing parallel requests
URL: https://github.com/apache/metron/pull/1514#discussion_r327004808
##
File path:
metron-interface/metron-alerts/src/app/alerts/alerts-list/alerts-list.component.ts
##
@@ -282,15 +324,6 @@ export class AlertsListComponent implements OnInit,
OnDestroy {
this.search();
}
- onPausePlay() {
-this.isRefreshPaused = !this.isRefreshPaused;
-if (this.isRefreshPaused) {
- this.tryStopPolling();
-} else {
- this.search(false);
-}
- }
-
Review comment:
Logic moved to auto-polling.service and component.
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
With regards,
Apache Git Services
[GitHub] [metron] tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests
tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts
UI: Indicating loading and preventing parallel requests
URL: https://github.com/apache/metron/pull/1514#discussion_r327001281
##
File path:
metron-interface/metron-alerts/src/app/alerts/alerts-list/alerts-list.component.ts
##
@@ -226,8 +259,6 @@ export class AlertsListComponent implements OnInit,
OnDestroy {
onClear() {
this.timeStampFilterPresent = false;
this.queryBuilder.clearSearch();
-if (this.hideQueryBuilder) { this.manualQuery.nativeElement.value = '*'; }
Review comment:
Source of manual query string is no longer the native dom element. I moved
it to query builder to eliminate the two posibble source of query. Logic
belongs to query creations is inside query builder to encapsulate and not make
alerts-list more bloated.
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
With regards,
Apache Git Services
[GitHub] [metron] tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests
tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts
UI: Indicating loading and preventing parallel requests
URL: https://github.com/apache/metron/pull/1514#discussion_r327021709
##
File path: metron-interface/metron-alerts/src/app/utils/httpUtil.ts
##
@@ -33,9 +33,12 @@ export class HttpUtil {
return body || {};
}
+ /**
+ * @depricated Turning all errors to 404 and hiding actual errors from the
consumers
+ * could limit how we can recover or react to errors.
+ * Use sessionExpiration instead and/or introduce new composable
error handlers.
+ */
public static handleError(res: HttpErrorResponse): Observable {
-// In a real world app, we might use a remote logging infrastructure
-// We'd also dig deeper into the error to get a better message
Review comment:
If we once accepted to have comments like this in the code using // FIXME is
also acceptable. :)
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
With regards,
Apache Git Services
[GitHub] [metron] tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests
tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests URL: https://github.com/apache/metron/pull/1514#discussion_r326992592 ## File path: metron-interface/metron-alerts/src/app/alerts/alerts-list/alerts-list.component.html ## @@ -84,14 +92,20 @@ - + - - Data is in a stale state! Click - to update your view based on your current filter and time-range configuration! - Review comment: The data state related warning messages was refactored to functions. 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 With regards, Apache Git Services
[GitHub] [metron] tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests
tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts
UI: Indicating loading and preventing parallel requests
URL: https://github.com/apache/metron/pull/1514#discussion_r327005089
##
File path:
metron-interface/metron-alerts/src/app/alerts/alerts-list/alerts-list.component.ts
##
@@ -460,69 +472,30 @@ export class AlertsListComponent implements OnInit,
OnDestroy {
}
showConfigureTable() {
-this.saveRefreshState();
+this.autoPollingSvc.setSuppression(true);
this.router.navigateByUrl('/alerts-list(dialog:configure-table)');
}
showDetails(alert: Alert) {
this.selectedAlerts = [];
this.selectedAlerts = [alert];
-this.saveRefreshState();
+this.autoPollingSvc.setSuppression(true);
let sourceType = alert.source[this.globalConfig['source.type.field']];
let url = '/alerts-list(dialog:details/' + sourceType + '/' +
alert.source.guid + '/' + alert.index + ')';
this.router.navigateByUrl(url);
}
- saveRefreshState() {
-this.lastIsRefreshPausedValue = this.isRefreshPaused;
-this.tryStopPolling();
- }
-
- pause() {
-this.isRefreshPaused = true;
-this.tryStopPolling();
- }
-
- resume() {
-this.isRefreshPaused = false;
-this.tryStartPolling();
- }
-
showSavedSearches() {
Review comment:
Moved to auto-polling.service/component.
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
With regards,
Apache Git Services
[GitHub] [metron] tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests
tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts
UI: Indicating loading and preventing parallel requests
URL: https://github.com/apache/metron/pull/1514#discussion_r326999623
##
File path:
metron-interface/metron-alerts/src/app/alerts/alerts-list/alerts-list.component.ts
##
@@ -170,7 +185,7 @@ export class AlertsListComponent implements OnInit,
OnDestroy {
}
getAlertColumnNames(resetPaginationForSearch: boolean) {
-observableForkJoin(
+forkJoin(
Review comment:
In some places in the code we just renamed standard rxjs operators. Don't
think too much explanation needed here. I changed the code to use the standard
names.
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
With regards,
Apache Git Services
[GitHub] [metron] tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests
tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts
UI: Indicating loading and preventing parallel requests
URL: https://github.com/apache/metron/pull/1514#discussion_r327009938
##
File path:
metron-interface/metron-alerts/src/app/alerts/alerts-list/auto-polling/auto-polling.service.ts
##
@@ -0,0 +1,184 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+import { Injectable } from '@angular/core';
+import { Subscription, Subject, Observable, interval, onErrorResumeNext } from
'rxjs';
+import { SearchService } from 'app/service/search.service';
+import { QueryBuilder } from '../query-builder';
+import { SearchResponse } from 'app/model/search-response';
+import { switchMap, filter, takeWhile, tap } from 'rxjs/operators';
+import { POLLING_DEFAULT_STATE } from 'app/utils/constants';
+import { RestError } from 'app/model/rest-error';
+import { DialogType } from 'app/shared/metron-dialog/metron-dialog.component';
+import { DialogService } from 'app/service/dialog.service';
+
+interface AutoPollingStateModel {
+ isActive: boolean,
+ refreshInterval: number,
+}
+
+@Injectable()
+export class AutoPollingService {
Review comment:
Auto polling logic was previously distributed across alerts-list,
query-builder, and search.service.
I was refactored them out and encapsulated in a dedicated service.
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
With regards,
Apache Git Services
[GitHub] [metron] tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests
tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts
UI: Indicating loading and preventing parallel requests
URL: https://github.com/apache/metron/pull/1514#discussion_r327020559
##
File path:
metron-interface/metron-alerts/src/app/shared/modal-loading-indicator/modal-loading-indicator.component.scss
##
@@ -15,7 +15,26 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-"use strict";
-exports.environment = {
-production: true
-};
Review comment:
Git get confused on this.
environment.prod.js was deleted as an unused file
scss was introduced for the modal loader.
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
With regards,
Apache Git Services
[GitHub] [metron] tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests
tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts
UI: Indicating loading and preventing parallel requests
URL: https://github.com/apache/metron/pull/1514#discussion_r326993769
##
File path:
metron-interface/metron-alerts/src/app/alerts/alerts-list/alerts-list.component.scss
##
@@ -197,27 +197,6 @@ $searchbox-height: 42px;
cursor: pointer;
}
-.pause-play {
- height: 38px;
- padding: 0px;
- border: 1px solid #0F6F9E;
- width: 38px;
- line-height: 39px;
- border-radius: 12px;
- margin-left: 15px;
- background: $mine-shaft-2;
- cursor: pointer;
-
- i {
-font-size: 17px;
-color: $piction-blue;
- }
-
- .fa-play {
-padding-left: 3px;
- }
-}
-
Review comment:
alerts-list.component is extremely big. I introduced an
auto-polling.component to encapsulate visual aspect of auto polling and make
alerts-list little bit smaller.
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
With regards,
Apache Git Services
[GitHub] [metron] tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests
tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts
UI: Indicating loading and preventing parallel requests
URL: https://github.com/apache/metron/pull/1514#discussion_r327019632
##
File path:
metron-interface/metron-alerts/src/app/service/elasticsearch-localstorage-impl.ts
##
@@ -20,6 +16,8 @@ import {catchError, map, onErrorResumeNext} from
'rxjs/operators';
* limitations under the License.
*/
import {Observable} from 'rxjs';
+import {throwError as observableThrowError} from 'rxjs';
+import {catchError, map, onErrorResumeNext} from 'rxjs/operators';
Review comment:
Moved imports after the license header.
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
With regards,
Apache Git Services
[GitHub] [metron] tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests
tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts
UI: Indicating loading and preventing parallel requests
URL: https://github.com/apache/metron/pull/1514#discussion_r327017385
##
File path:
metron-interface/metron-alerts/src/app/alerts/configure-rows/configure-rows.component.ts
##
@@ -16,53 +16,29 @@
* limitations under the License.
*/
import { Component, Input, HostListener, ElementRef, Output, EventEmitter }
from '@angular/core';
-import { TableMetadata } from '../../model/table-metadata';
-import { ConfigureTableService } from '../../service/configure-table.service';
+export interface ConfigureRowsModel {
+ values: {
+pageSize: number;
+refreshInterval: number;
+ },
+ triggerQuery: boolean
+}
@Component({
selector: 'app-configure-rows',
templateUrl: './configure-rows.component.html',
styleUrls: ['./configure-rows.component.scss']
})
export class ConfigureRowsComponent {
-
showView = false;
- tableMetadata = new TableMetadata();
@Input() srcElement: HTMLElement;
- @Output() sizeChange = new EventEmitter();
- @Output() intervalChange = new EventEmitter();
- @Output() configRowsChange = new EventEmitter();
-
- constructor(private elementRef: ElementRef,
- private configureTableService: ConfigureTableService) {}
-
- @Input()
- get size() {
-return this.tableMetadata.size;
- }
+ @Input() pageSize: number;
+ @Input() refreshInterval: number;
- set size(val) {
-this.tableMetadata.size = val;
- }
+ @Output() configRowsChange = new EventEmitter();
- @Input()
- get interval() {
-return this.tableMetadata.refreshInterval;
- }
-
- set interval(val) {
-this.tableMetadata.refreshInterval = val;
- }
-
- @Input()
- get tableMetaData() {
-return this.tableMetadata;
- }
-
- set tableMetaData(val) {
Review comment:
tableMetaData is no directly used by this component so I refactored it out.
Changes here was needed to separate config change events we like to trigger
query from others.
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
With regards,
Apache Git Services
[GitHub] [metron] tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests
tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts
UI: Indicating loading and preventing parallel requests
URL: https://github.com/apache/metron/pull/1514#discussion_r327021175
##
File path: metron-interface/metron-alerts/src/app/utils/constants.ts
##
@@ -36,7 +36,7 @@ export const CUSTOMM_DATE_RANGE_LABEL = 'Date Range';
export const TREE_SUB_GROUP_SIZE = 5;
export const INDEXES = environment.indices ? environment.indices.split(',') :
[];
-export const POLLING_DEFAULT_STATE = !environment.defaultPollingState;
Review comment:
The logic here was a bit confused. It was fixed and aligned everywhere in
the code.
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
With regards,
Apache Git Services
[GitHub] [metron] tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests
tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts
UI: Indicating loading and preventing parallel requests
URL: https://github.com/apache/metron/pull/1514#discussion_r327003814
##
File path:
metron-interface/metron-alerts/src/app/alerts/alerts-list/alerts-list.component.ts
##
@@ -271,9 +298,24 @@ export class AlertsListComponent implements OnInit,
OnDestroy {
this.staleDataState = true;
}
- onConfigRowsChange() {
-this.searchService.interval = this.refreshInterval;
-this.search();
Review comment:
onConfigRowChange was triggering a manual query. Auto polling interval
setting is part of row config. It make no sense to trigger a manual query when
user change a settings belong to auto polling. I was separated this.
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
With regards,
Apache Git Services
[GitHub] [metron] tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests
tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts
UI: Indicating loading and preventing parallel requests
URL: https://github.com/apache/metron/pull/1514#discussion_r327003056
##
File path:
metron-interface/metron-alerts/src/app/alerts/alerts-list/alerts-list.component.ts
##
@@ -258,11 +289,7 @@ export class AlertsListComponent implements OnInit,
OnDestroy {
alert => (alert.source.metron_alert && alert.source.metron_alert.length
> 0)
);
-if (selectedAlerts.length > 0) {
- this.pause();
-} else {
- this.resume();
-}
+this.autoPollingSvc.setSuppression(!!selectedAlerts.length);
Review comment:
The concept of suppression was added to auto polling to prevent starting and
sopping it when ever another feature interfere whit it. With this we no longer
resetting the timer, persistent state only turning a flag on and skipping
polling cycles til it's needed. (the list of features suppressing auto polling
is in the pr desc)
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
With regards,
Apache Git Services
[GitHub] [metron] tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests
tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts
UI: Indicating loading and preventing parallel requests
URL: https://github.com/apache/metron/pull/1514#discussion_r327019294
##
File path: metron-interface/metron-alerts/src/app/model/table-metadata.ts
##
@@ -15,23 +15,17 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-import {PageSize, RefreshInterval} from
'../alerts/configure-rows/configure-rows-enums';
-import {ColumnMetadata} from './column-metadata';
+import { PageSize } from '../alerts/configure-rows/configure-rows-enums';
+import { ColumnMetadata } from './column-metadata';
export class TableMetadata {
size = PageSize.TWENTY_FIVE;
- refreshInterval = RefreshInterval.TEN_MIN;
- hideResolvedAlerts = true;
- hideDismissedAlerts = true;
Review comment:
hideResolvedAlerts = true;
hideDismissedAlerts = true;
were dead code.
refreshInterval was moved to auto-polling.service
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
With regards,
Apache Git Services
[GitHub] [metron] tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests
tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts
UI: Indicating loading and preventing parallel requests
URL: https://github.com/apache/metron/pull/1514#discussion_r326996827
##
File path:
metron-interface/metron-alerts/src/app/alerts/alerts-list/alerts-list.component.ts
##
@@ -62,10 +63,7 @@ export class AlertsListComponent implements OnInit,
OnDestroy {
alerts: Alert[] = [];
searchResponse: SearchResponse = new SearchResponse();
colNumberTimerId: number;
- refreshInterval = RefreshInterval.TEN_MIN;
- refreshTimer: Subscription;
- isRefreshPaused = POLLING_DEFAULT_STATE;
- lastIsRefreshPausedValue = false;
Review comment:
Auto polling related this moved to auto-polling.component or
auto-polling.service to keep this functionality encapsulated and shrink the
size of alerts-list.
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
With regards,
Apache Git Services
[GitHub] [metron] tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests
tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts
UI: Indicating loading and preventing parallel requests
URL: https://github.com/apache/metron/pull/1514#discussion_r327004201
##
File path:
metron-interface/metron-alerts/src/app/alerts/alerts-list/alerts-list.component.ts
##
@@ -386,31 +410,19 @@ export class AlertsListComponent implements OnInit,
OnDestroy {
this.setSearchRequestSize();
-if (this.hideQueryBuilder) {
- const newSearch = new SearchRequest();
- newSearch.query = this.manualQuery.nativeElement.value;
- newSearch.size = this.pagination.size;
- newSearch.from = 0;
-
- this.searchService.search(newSearch).subscribe(results => {
-this.setData(results);
-this.staleDataState = false;
- }, error => {
-this.setData(new SearchResponse());
-
this.dialogService.launchDialog(ElasticsearchUtils.extractESErrorMessage(error),
DialogType.Error);
- });
-
- this.tryStartPolling(newSearch);
-} else {
-
this.searchService.search(this.queryBuilder.searchRequest).subscribe(results =>
{
+this.pendingSearch =
this.searchService.search(this.queryBuilder.searchRequest).subscribe(
+ results => {
this.setData(results);
+this.pendingSearch = null;
this.staleDataState = false;
}, error => {
this.setData(new SearchResponse());
-
this.dialogService.launchDialog(ElasticsearchUtils.extractESErrorMessage(error),
DialogType.Error);
+this.pendingSearch = null;
+this.dialogService.launchDialog('Server were unable to apply query
string.', DialogType.Error);
});
- this.tryStartPolling();
+if (this.autoPollingSvc.getIsPollingActive()) {
+ this.autoPollingSvc.dropNextAndContinue();
Review comment:
This was a large block of code duplication.
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
With regards,
Apache Git Services
[GitHub] [metron] tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests
tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests URL: https://github.com/apache/metron/pull/1514#discussion_r327009244 ## File path: metron-interface/metron-alerts/src/app/alerts/alerts-list/auto-polling/auto-polling.component.html ## @@ -0,0 +1,17 @@ + + + + + Review comment: Auto polling UI controls refactored to a separated component. 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 With regards, Apache Git Services
[GitHub] [metron] tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests
tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts
UI: Indicating loading and preventing parallel requests
URL: https://github.com/apache/metron/pull/1514#discussion_r326997272
##
File path:
metron-interface/metron-alerts/src/app/alerts/alerts-list/alerts-list.component.ts
##
@@ -75,7 +73,18 @@ export class AlertsListComponent implements OnInit,
OnDestroy {
@ViewChild('table') table: ElementRef;
@ViewChild('dataViewComponent') dataViewComponent: TableViewComponent;
@ViewChild(AlertSearchDirective) alertSearchDirective: AlertSearchDirective;
- @ViewChild('manualQuery') manualQuery: ElementRef;
+
+ private manualQueryFieldChangeSubs: Subscription;
+ private manualQueryInputEl: ElementRef;
+ @ViewChild('manualQuery') set manualQuery(el: ElementRef) {
+if (el && !this.manualQueryInputEl) {
+ this.manualQueryInputEl = el;
+ this.manualQueryFieldChangeSubs =
this.addManualQueryFieldChangeStream(el.nativeElement);
+}
+ };
+ get manualQuery(): ElementRef {
+return this.manualQueryInputEl;
+ }
Review comment:
This structure is used to use to deal with a viewChild in an ngIf.
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
With regards,
Apache Git Services
[GitHub] [metron] tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests
tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts
UI: Indicating loading and preventing parallel requests
URL: https://github.com/apache/metron/pull/1514#discussion_r326995509
##
File path:
metron-interface/metron-alerts/src/app/alerts/alerts-list/alerts-list.component.spec.ts
##
@@ -28,53 +28,136 @@ import { SaveSearchService } from
'app/service/save-search.service';
import { MetaAlertService } from 'app/service/meta-alert.service';
import { GlobalConfigService } from 'app/service/global-config.service';
import { DialogService } from 'app/service/dialog.service';
-import { SearchRequest } from 'app/model/search-request';
-import { Observable, of, Subject } from 'rxjs';
-import { Filter } from 'app/model/filter';
-import { QueryBuilder } from './query-builder';
import { TIMESTAMP_FIELD_NAME } from 'app/utils/constants';
-import { SearchResponse } from 'app/model/search-response';
import { By } from '@angular/platform-browser';
+import { Observable, Subject, of, throwError } from 'rxjs';
+import { Filter } from 'app/model/filter';
+import { QueryBuilder, FilteringMode } from './query-builder';
+import { SearchResponse } from 'app/model/search-response';
+import { AutoPollingService } from './auto-polling/auto-polling.service';
+import { Router } from '@angular/router';
+import { Alert } from 'app/model/alert';
+import { AlertSource } from 'app/model/alert-source';
+import { SearchRequest } from 'app/model/search-request';
+import { query } from '@angular/core/src/render3';
+import { RestError } from 'app/model/rest-error';
+import { DialogType } from 'app/shared/metron-dialog/metron-dialog.component';
+
+@Component({
+ selector: 'app-auto-polling',
+ template: '',
+})
+class MockAutoPollingComponent {}
+
+@Component({
+ selector: 'app-configure-rows',
+ template: '',
+})
+class MockConfigureRowsComponent {
+ @Input() refreshInterval = 0;
+ @Input() srcElement = {};
+ @Input() pageSize = 0;
+}
+
+@Component({
+ selector: 'app-modal-loading-indicator',
+ template: '',
+})
+class MockModalLoadingIndicatorComponent {
+ @Input() show = false;
+}
+
+@Component({
+ selector: 'app-time-range',
+ template: '',
+})
+class MockTimeRangeComponent {
+ @Input() disabled = false;
+ @Input() selectedTimeRange = {};
+}
+
+@Directive({
+ selector: '[appAceEditor]',
+})
+class MockAceEditorDirective {
+ @Input() text = '';
+}
+
+@Component({
+ selector: 'app-alert-filters',
+ template: '',
+})
+class MockAlertFilterComponent {
+ @Input() facets = [];
+}
+
+@Component({
+ selector: 'app-group-by',
+ template: '',
+})
+class MockGroupByComponent {
+ @Input() facets = [];
+}
+
+@Component({
+ selector: 'app-table-view',
+ template: '',
+})
+class MockTableViewComponent {
+ @Input() alerts = [];
+ @Input() pagination = {};
+ @Input() alertsColumnsToDisplay = [];
+ @Input() selectedAlerts = [];
+}
+
+@Component({
+ selector: 'app-tree-view',
+ template: '',
+})
+class MockTreeViewComponent {
+ @Input() alerts = [];
+ @Input() pagination = {};
+ @Input() alertsColumnsToDisplay = [];
+ @Input() selectedAlerts = [];
+ @Input() globalConfig = {};
+ @Input() query = '';
+ @Input() groups = [];
+}
+
describe('AlertsListComponent', () => {
let component: AlertsListComponent;
let fixture: ComponentFixture;
- let searchServiceStub = {
-search() { return of({
- total: 0,
- groupedBy: '',
- results: [],
- facetCounts: [],
- groups: []
-}) },
-pollSearch() { return of({}) }
- }
- let queryBuilderStub = {
-addOrUpdateFilter() { return {} },
-clearSearch() { return {} },
-generateSelect() { return '*' },
-isTimeStampFieldPresent() { return {} },
-filters: [{}],
-searchRequest: {
- from: 0
-}
- }
Review comment:
I removed this kind of a third type of mocking to when we use variables to
define mocks. In other places in our code base we used to use mock classes and
inline mocks in our import declaration.
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
With regards,
Apache Git Services
[GitHub] [metron] tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests
tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts
UI: Indicating loading and preventing parallel requests
URL: https://github.com/apache/metron/pull/1514#discussion_r327019948
##
File path: metron-interface/metron-alerts/src/app/service/search.service.ts
##
@@ -79,21 +76,12 @@ export class SearchService {
catchError(HttpUtil.handleError));
}
- public pollSearch(searchRequest: SearchRequest): Observable {
-return this.ngZone.runOutsideAngular(() => {
- return this.ngZone.run(() => {
-return observableInterval(this.interval * 1000).pipe(switchMap(() => {
- return this.search(searchRequest);
-}));
- });
-});
- }
-
Review comment:
polling interval logic was refactored to auto-polling.service.
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
With regards,
Apache Git Services
[GitHub] [metron] tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests
tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests URL: https://github.com/apache/metron/pull/1514#discussion_r326992280 ## File path: metron-interface/metron-alerts/src/app/alerts/alerts-list/alerts-list.component.html ## @@ -12,33 +12,37 @@ the specific language governing permissions and limitations under the License. Review comment: Only minor refactoring happened this file to make it more clear. 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 With regards, Apache Git Services
[GitHub] [metron] tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests
tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests URL: https://github.com/apache/metron/pull/1514#discussion_r326991804 ## File path: metron-interface/metron-alerts/cypress/integration/search/auto-polling.feature.spec.js ## @@ -0,0 +1,98 @@ +/// Review comment: Cypress test: high level UAT for auto polling. 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 With regards, Apache Git Services
[GitHub] [metron] tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests
tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts
UI: Indicating loading and preventing parallel requests
URL: https://github.com/apache/metron/pull/1514#discussion_r326991079
##
File path: metron-interface/metron-alerts/cypress/fixtures/search-1.1.json
##
@@ -0,0 +1,102 @@
+{
Review comment:
Cypress test: mock data.
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
With regards,
Apache Git Services
[GitHub] [metron] tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts UI: Indicating loading and preventing parallel requests
tiborm commented on a change in pull request #1514: METRON-2190: [UI] Alerts
UI: Indicating loading and preventing parallel requests
URL: https://github.com/apache/metron/pull/1514#discussion_r326991248
##
File path: metron-interface/metron-alerts/cypress/fixtures/search-1.2.json
##
@@ -0,0 +1,102 @@
+{
Review comment:
Cypress test: mock data.
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
With regards,
Apache Git Services
