Skip to content

Commit 8f00651

Browse files
isaachillygraduta
andauthored
[O2B-1528] Split GAQ summary requests (#2071)
* Re-enabled GAQ Summaries * Added runNumber filtering for GAQ summaries * Use RemoteDataSource and AbortController to make per-run GAQ summary fetches cancellable. --------- Co-authored-by: George R. <george.raduta@cern.ch>
1 parent 9e3f539 commit 8f00651

File tree

7 files changed

+222
-27
lines changed

7 files changed

+222
-27
lines changed

lib/database/repositories/QcFlagRepository.js

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -105,9 +105,10 @@ class QcFlagRepository extends Repository {
105105
* and informtion about missing and unverified flags
106106
*
107107
* @param {number} dataPassId the id of a data-pass
108+
* @param {number} [runNumber] run number to filter by
108109
* @return {Promise<Object.<number, RunGaqSubSummary>>} resolves with the map between run number and the corresponding run GAQ summary
109110
*/
110-
async getGaqCoverages(dataPassId) {
111+
async getGaqCoverages(dataPassId, runNumber) {
111112
const blockAggregationQuery = `
112113
SELECT
113114
gaq_periods.data_pass_id,
@@ -147,7 +148,8 @@ class QcFlagRepository extends Repository {
147148
AND (qcfep.to IS NULL OR qcfep.\`to\` > gaq_periods.\`from\`)
148149
149150
WHERE gaq_periods.data_pass_id = :dataPassId
150-
GROUP BY gaq_periods.data_pass_id, gaq_periods.run_number, gaq_periods.\`from\`, gaq_periods.to
151+
${runNumber !== undefined && runNumber !== null ? 'AND gaq_periods.run_number = :runNumber' : ''}
152+
GROUP BY gaq_periods.data_pass_id, gaq_periods.run_number, gaq_periods.\`from\`, gaq_periods.\`to\`
151153
`;
152154

153155
const summaryQuery = `
@@ -164,14 +166,20 @@ class QcFlagRepository extends Repository {
164166
FROM (${blockAggregationQuery}) AS gaq
165167
GROUP BY gaq.data_pass_id, gaq.run_number;
166168
`;
167-
const [rows] = await this.model.sequelize.query(summaryQuery, { replacements: { dataPassId } });
169+
170+
const replacements = { dataPassId };
171+
if (runNumber !== undefined && runNumber !== null) {
172+
replacements.runNumber = runNumber;
173+
}
174+
175+
const [rows] = await this.model.sequelize.query(summaryQuery, { replacements });
168176
const entries = rows.map(({
169177
run_number,
170178
bad_coverage,
171179
mcr_coverage,
172180
good_coverage,
173181
flags_list,
174-
verifiedd_flags_list,
182+
verified_flags_list,
175183
undefined_quality_periods_count,
176184
}) => [
177185
run_number,
@@ -180,7 +188,7 @@ class QcFlagRepository extends Repository {
180188
mcReproducibleCoverage: parseFloat(mcr_coverage ?? '0'),
181189
goodCoverage: parseFloat(good_coverage ?? '0'),
182190
flagsIds: [...new Set(flags_list?.split(','))],
183-
verifiedFlagsIds: [...new Set(verifiedd_flags_list?.split(','))],
191+
verifiedFlagsIds: [...new Set(verified_flags_list?.split(','))],
184192
undefinedQualityPeriodsCount: undefined_quality_periods_count,
185193
},
186194
]);

lib/public/views/Runs/RunPerDataPass/RunsPerDataPassOverviewModel.js

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import { SkimmingStage } from '../../../domain/enums/SkimmingStage.js';
2222
import { NumericalComparisonFilterModel } from '../../../components/Filters/common/filters/NumericalComparisonFilterModel.js';
2323
import { jsonFetch } from '../../../utilities/fetch/jsonFetch.js';
2424
import { mergeRemoteData } from '../../../utilities/mergeRemoteData.js';
25+
import { RemoteDataSource } from '../../../utilities/fetch/RemoteDataSource.js';
2526
import { DetectorType } from '../../../domain/enums/DetectorTypes.js';
2627

2728
const ALL_CPASS_PRODUCTIONS_REGEX = /cpass\d+/;
@@ -58,6 +59,12 @@ export class RunsPerDataPassOverviewModel extends FixedPdpBeamTypeRunsOverviewMo
5859
this._markAsSkimmableRequestResult$ = new ObservableData(RemoteData.notAsked());
5960
this._markAsSkimmableRequestResult$.bubbleTo(this);
6061

62+
this._gaqSummary$ = new ObservableData({});
63+
this._gaqSummary$.bubbleTo(this);
64+
65+
this._gaqSummarySources = {};
66+
this._gaqSequenceAbortController = null;
67+
6168
this._skimmableRuns$ = new ObservableData(RemoteData.notAsked());
6269
this._skimmableRuns$.bubbleTo(this);
6370

@@ -71,6 +78,8 @@ export class RunsPerDataPassOverviewModel extends FixedPdpBeamTypeRunsOverviewMo
7178

7279
this._discardAllQcFlagsActionState$ = new ObservableData(RemoteData.notAsked());
7380
this._discardAllQcFlagsActionState$.bubbleTo(this);
81+
82+
this._item$.observe(() => this._fetchGaqSummaryForCurrentRuns());
7483
}
7584

7685
/**
@@ -321,6 +330,83 @@ export class RunsPerDataPassOverviewModel extends FixedPdpBeamTypeRunsOverviewMo
321330
}
322331
}
323332

333+
/**
334+
* Cancel all ongoing and future GAQ summary fetches
335+
* @return {void} promise
336+
*/
337+
_abortGaqFetches() {
338+
// Aborts the overall sequence fetch, i.e. stops further individual run fetches
339+
this._gaqSequenceAbortController?.abort();
340+
341+
// Aborts individual run fetches in-flight
342+
for (const runNumber in this._gaqSummarySources) {
343+
this._gaqSummarySources[runNumber]?._abortController?.abort();
344+
}
345+
}
346+
347+
/**
348+
* Fetch GAQ summary for given data pass and run
349+
* @param {number} [runNumber] run number to filter by
350+
* @return {Promise<void>} resolves once data has been fetched
351+
*/
352+
async _fetchGaqSummary(runNumber) {
353+
this._gaqSummarySources[runNumber] = new RemoteDataSource();
354+
// Pipe the result into the correct slot in the gaqSummary$ observable
355+
this._gaqSummarySources[runNumber].pipe({
356+
setCurrent: (remoteData) => {
357+
const current = this._gaqSummary$.getCurrent();
358+
this._gaqSummary$.setCurrent({
359+
...current,
360+
[runNumber]: remoteData.apply({ Success: (response) => response.data }),
361+
});
362+
},
363+
});
364+
const url = buildUrl('/api/qcFlags/summary/gaq', {
365+
dataPassId: this._dataPassId,
366+
mcReproducibleAsNotBad: this._mcReproducibleAsNotBad,
367+
runNumber: runNumber,
368+
});
369+
await this._gaqSummarySources[runNumber].fetch(url);
370+
}
371+
372+
/**
373+
* Fetch GAQ summary for currently displayed (paginated) runs
374+
* @return {void}
375+
*/
376+
_fetchGaqSummaryForCurrentRuns() {
377+
// Stop any previous fetch (quickly changing filters, pagination, etc)
378+
this._abortGaqFetches();
379+
380+
// Reset abort controller
381+
this._gaqSequenceAbortController = new AbortController();
382+
const { signal } = this._gaqSequenceAbortController;
383+
384+
this._item$.getCurrent().match({
385+
Success: async (runs) => {
386+
const runNumbers = runs.map((run) => run.runNumber);
387+
388+
// Prepare GAQ summary object with NotAsked RemoteData state for all runs
389+
let gaqSummary = {};
390+
for (const runNumber of runNumbers) {
391+
gaqSummary = { ...gaqSummary, [runNumber]: RemoteData.notAsked() };
392+
}
393+
this._gaqSummary$.setCurrent(gaqSummary);
394+
395+
// Trigger GAQ summary fetch for each run
396+
for (const runNumber of runNumbers) {
397+
if (signal.aborted) {
398+
return;
399+
}
400+
401+
await this._fetchGaqSummary(runNumber);
402+
}
403+
},
404+
Other: () => {
405+
// Don't fetch if runs haven't loaded successfully yet
406+
},
407+
});
408+
}
409+
324410
/**
325411
* Fetch skimmable runs for given data pass
326412
* @return {Promise<void>} resolves once data are fetched

lib/public/views/Runs/RunPerDataPass/RunsPerDataPassOverviewPage.js

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import { runNumbersFilter } from '../../../components/Filters/RunsFilter/runNumb
2525
import { qcSummaryLegendTooltip } from '../../../components/qcFlags/qcSummaryLegendTooltip.js';
2626
import { isRunNotSubjectToQc } from '../../../components/qcFlags/isRunNotSubjectToQc.js';
2727
import { frontLink } from '../../../components/common/navigation/frontLink.js';
28+
import { getQcSummaryDisplay } from '../ActiveColumns/getQcSummaryDisplay.js';
2829
import errorAlert from '../../../components/common/errorAlert.js';
2930
import { switchInput } from '../../../components/common/form/switchInput.js';
3031
import { PdpBeamType } from '../../../domain/enums/PdpBeamType.js';
@@ -102,6 +103,7 @@ export const RunsPerDataPassOverviewPage = ({
102103
detectors: remoteDetectors,
103104
dataPass: remoteDataPass,
104105
qcSummary: remoteQcSummary,
106+
gaqSummary: remoteGaqSummary,
105107
displayOptions,
106108
dataPassId,
107109
sortModel,
@@ -118,6 +120,9 @@ export const RunsPerDataPassOverviewPage = ({
118120

119121
return h(
120122
'.intermediate-flex-column',
123+
{ onremove: () => {
124+
perDataPassOverviewModel._abortGaqFetches();
125+
} },
121126
mergeRemoteData([remoteDataPass, remoteRuns, remoteDetectors, remoteQcSummary]).match({
122127
NotAsked: () => null,
123128
Failure: (errors) => errorAlert(errors),
@@ -160,14 +165,21 @@ export const RunsPerDataPassOverviewPage = ({
160165
),
161166
visible: true,
162167
format: (_, { runNumber }) => {
163-
const gaqDisplay = h('button.btn.btn-primary.w-100', [
164-
'GAQ',
165-
h(
166-
'.d-inline-block.va-t-bottom',
167-
tooltip(h('.f7', iconWarning()), 'GAQ Summary is disabled, please click to view GAQ flags'),
168-
),
169-
]);
170-
return frontLink(gaqDisplay, 'gaq-flags', { dataPassId, runNumber });
168+
const gaqLoadingSpinner = h('.flex-row.items-center.justify-center.black', spinner({ size: 2, absolute: false }));
169+
const runGaqSummary = remoteGaqSummary[runNumber];
170+
171+
return runGaqSummary.match({
172+
Success: (gaqSummary) => {
173+
const gaqDisplay = gaqSummary?.undefinedQualityPeriodsCount === 0
174+
? getQcSummaryDisplay(gaqSummary)
175+
: h('button.btn.btn-primary.w-100', 'GAQ');
176+
177+
return frontLink(gaqDisplay, 'gaq-flags', { dataPassId, runNumber });
178+
},
179+
Loading: () => tooltip(gaqLoadingSpinner, 'Loading GAQ summary...'),
180+
NotAsked: () => tooltip(gaqLoadingSpinner, 'Loading GAQ summary...'),
181+
Failure: () => tooltip(iconWarning(), 'Failed to load GAQ summary'),
182+
});
171183
},
172184
filter: ({ filteringModel }) => numericalComparisonFilter(
173185
filteringModel.get('gaq[notBadFraction]'),

lib/server/controllers/qcFlag.controller.js

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -382,17 +382,18 @@ const getGaqQcFlagsHandler = async (request, response) => {
382382
const getGaqSummaryHandler = async (request, response) => {
383383
const validatedDTO = await dtoValidator(
384384
DtoFactory.queryOnly(Joi.object({
385-
dataPassId: Joi.number().required(),
385+
dataPassId: Joi.number().positive().required(),
386386
mcReproducibleAsNotBad: Joi.boolean().optional(),
387+
runNumber: Joi.number().positive().required(),
387388
})),
388389
request,
389390
response,
390391
);
391392
if (validatedDTO) {
392393
try {
393-
const { dataPassId, mcReproducibleAsNotBad = false } = validatedDTO.query;
394+
const { dataPassId, mcReproducibleAsNotBad = false, runNumber } = validatedDTO.query;
394395

395-
const data = await gaqService.getSummary(dataPassId, { mcReproducibleAsNotBad });
396+
const data = await gaqService.getSummary(dataPassId, { mcReproducibleAsNotBad, runNumber });
396397
response.json({ data });
397398
} catch (error) {
398399
updateExpressResponseFromNativeError(response, error);

lib/server/services/qualityControlFlag/GaqService.js

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,13 +44,14 @@ class GaqService {
4444
* @param {object} [options] additional options
4545
* @param {boolean} [options.mcReproducibleAsNotBad = false] if set to true,
4646
* `Limited Acceptance MC Reproducible` flag type is treated as good one
47+
* @param {number} [options.runNumber] Optional run number to filter by
4748
* @return {Promise<GaqSummary>} Resolves with the GAQ Summary
4849
*/
49-
async getSummary(dataPassId, { mcReproducibleAsNotBad = false } = {}) {
50+
async getSummary(dataPassId, { mcReproducibleAsNotBad = false, runNumber } = {}) {
5051
await getOneDataPassOrFail({ id: dataPassId });
51-
const gaqCoverages = await QcFlagRepository.getGaqCoverages(dataPassId);
52+
const gaqCoverages = await QcFlagRepository.getGaqCoverages(dataPassId, runNumber);
5253
const gaqSummary = Object.entries(gaqCoverages).map(([
53-
runNumber,
54+
runNumberMapped,
5455
{
5556
badCoverage,
5657
mcReproducibleCoverage,
@@ -60,7 +61,7 @@ class GaqService {
6061
undefinedQualityPeriodsCount,
6162
},
6263
]) => [
63-
runNumber,
64+
runNumberMapped,
6465
{
6566
[QcSummarProperties.BAD_EFFECTIVE_RUN_COVERAGE]: badCoverage + (mcReproducibleAsNotBad ? 0 : mcReproducibleCoverage),
6667
[QcSummarProperties.EXPLICITELY_NOT_BAD_EFFECTIVE_RUN_COVERAGE]:
@@ -71,6 +72,14 @@ class GaqService {
7172
},
7273
]);
7374

75+
/**
76+
* If runNumber is specified, only one summary is returned but the getGaqCoverages
77+
* returns still with runNumber as key, so we extract the single value from the array.
78+
*/
79+
if (runNumber && gaqSummary.length === 1) {
80+
return Object.fromEntries(gaqSummary)[runNumber];
81+
}
82+
7483
return Object.fromEntries(gaqSummary);
7584
}
7685

test/api/qcFlags.test.js

Lines changed: 45 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -556,22 +556,62 @@ module.exports = () => {
556556
relations,
557557
);
558558

559-
const response = await request(server).get('/api/qcFlags/summary/gaq?dataPassId=3');
559+
const response = await request(server).get('/api/qcFlags/summary/gaq?dataPassId=3&runNumber=54');
560560
expect(response.status).to.be.equal(200);
561561
const { body: { data } } = response;
562562
expect(data).to.be.eql({
563-
54: {
564563
missingVerificationsCount: 1,
565564
mcReproducible: true,
566565
badEffectiveRunCoverage: 1,
567566
explicitlyNotBadEffectiveRunCoverage: 0,
568567
undefinedQualityPeriodsCount: 0,
569568
},
570-
});
569+
);
571570
});
572571

573-
it('should return 400 when bad query parameter provided', async () => {
574-
const response = await request(server).get('/api/qcFlags/summary/gaq');
572+
it('should return empty GAQ summary if no data exists for given dataPassId & runNumber combination', async () => {
573+
const response = await request(server).get('/api/qcFlags/summary/gaq?dataPassId=3&runNumber=999');
574+
expect(response.status).to.equal(200);
575+
const { body: { data } } = response;
576+
expect(data).to.eql({});
577+
});
578+
579+
it('should return 400 if dataPassId is not positive', async () => {
580+
const response = await request(server).get('/api/qcFlags/summary/gaq?dataPassId=-1&runNumber=54');
581+
expect(response.status).to.equal(400);
582+
expect(response.body.errors[0].detail).to.equal('"query.dataPassId" must be a positive number');
583+
});
584+
585+
it('should return 400 if runNumber is not positive', async () => {
586+
const response = await request(server).get('/api/qcFlags/summary/gaq?dataPassId=3&runNumber=-10');
587+
expect(response.status).to.equal(400);
588+
expect(response.body.errors[0].detail).to.equal('"query.runNumber" must be a positive number');
589+
});
590+
591+
it('should return 400 if dataPassId is not a number', async () => {
592+
const response = await request(server).get('/api/qcFlags/summary/gaq?dataPassId=abc&runNumber=54');
593+
expect(response.status).to.equal(400);
594+
const { errors } = response.body;
595+
expect(errors[0].detail).to.equal('"query.dataPassId" must be a number');
596+
});
597+
598+
it('should return 400 if runNumber is not a number', async () => {
599+
const response = await request(server).get('/api/qcFlags/summary/gaq?dataPassId=3&runNumber=abc');
600+
expect(response.status).to.equal(400);
601+
const { errors } = response.body;
602+
expect(errors[0].detail).to.equal('"query.runNumber" must be a number');
603+
});
604+
605+
it('should return 400 when runNumber parameter is missing', async () => {
606+
const response = await request(server).get('/api/qcFlags/summary/gaq?dataPassId=3');
607+
expect(response.status).to.be.equal(400);
608+
const { errors } = response.body;
609+
const titleError = errors.find((err) => err.source.pointer === '/data/attributes/query/runNumber');
610+
expect(titleError.detail).to.equal('"query.runNumber" is required');
611+
});
612+
613+
it('should return 400 when dataPassId parameter is missing', async () => {
614+
const response = await request(server).get('/api/qcFlags/summary/gaq?runNumber=54');
575615
expect(response.status).to.be.equal(400);
576616
const { errors } = response.body;
577617
const titleError = errors.find((err) => err.source.pointer === '/data/attributes/query/dataPassId');

0 commit comments

Comments
 (0)