Implement warnings table and frontend display (#1255)

ribalba · ArneTR · web-flow · commit 950f030e0e49 · 2025-07-31T17:31:48.000+02:00
* Store warnings from system checks

* Warnings must be saved after run as run_id is not set at the beginning of the run [skip ci]

* Reworked API to not include plain invalidated status, but count for invalidated

* Introduced a warning if the run was failed and breaking in API in compare

* Allowed warning route for DEFAULT user

* Reworked warnings display

* Warning string in run overview shall only show if one warning is present

* Test should now check for warning as system check warning are now transported to warnings and are always present in test scenarios [skip ci]

---------

Co-authored-by: Arne Tarara &lt;arne@datafuse.de&gt;
diff --git a/api/api_helpers.py b/api/api_helpers.py
@@ -180,7 +180,9 @@ def get_run_info(user, run_id):
                     LEFT JOIN categories as t on t.id = elements) as categories,
                 filename, start_measurement, end_measurement,
                 measurement_config, machine_specs, machine_id, usage_scenario, usage_scenario_variables,
-                created_at, invalid_run, phases, logs, failed, gmt_hash, runner_arguments
+                created_at,
+                (SELECT COUNT(id) FROM warnings as w WHERE w.run_id = runs.id) as invalid_run,
+                phases, logs, failed, gmt_hash, runner_arguments
             FROM runs
             WHERE
                 (TRUE = %s OR user_id = ANY(%s::int[]))
@@ -446,6 +448,19 @@ def determine_comparison_case(user, ids, force_mode=None):
 
     raise RuntimeError('Could not determine comparison case after checking all conditions')
 
+def check_run_failed(user, ids):
+    query = """
+            SELECT
+               COUNT(failed)
+            FROM runs
+            WHERE
+                (TRUE = %s OR user_id = ANY(%s::int[]))
+                AND id = ANY(%s::uuid[])
+                AND failed IS TRUE
+            """
+    params = (user.is_super_user(), user.visible_users(), ids)
+    return DB().fetch_one(query, params=params)[0]
+
 def get_phase_stats(user, ids):
     query = """
             SELECT
diff --git a/api/scenario_runner.py b/api/scenario_runner.py
@@ -14,7 +14,7 @@
 from api.object_specifications import Software, JobChange
 from api.api_helpers import (ORJSONResponseObjKeep, add_phase_stats_statistics,
                          determine_comparison_case,get_comparison_details,
-                         html_escape_multi, get_phase_stats, get_phase_stats_object,
+                         html_escape_multi, get_phase_stats, get_phase_stats_object, check_run_failed,
                          is_valid_uuid, convert_value, get_timeline_query,
                          get_run_info, get_machine_list, get_artifact, store_artifact,
                          authenticate, check_int_field_api)
@@ -172,6 +172,30 @@ async def get_notes(run_id, user: User = Depends(authenticate)):
     return ORJSONResponseObjKeep({'success': True, 'data': escaped_data})
 
 
+@router.get('/v1/warnings/{run_id}')
+async def get_warnings(run_id, user: User = Depends(authenticate)):
+    if run_id is None or not is_valid_uuid(run_id):
+        raise RequestValidationError('Run ID is not a valid UUID or empty')
+
+    query = '''
+            SELECT w.run_id, w.message, w.created_at
+            FROM warnings as w
+            JOIN runs as r on w.run_id = r.id
+            WHERE
+                (TRUE = %s OR r.user_id = ANY(%s::int[]))
+                AND w.run_id = %s
+            ORDER BY w.created_at DESC
+            '''
+
+    params = (user.is_super_user(), user.visible_users(), run_id)
+    data = DB().fetch_all(query, params=params)
+    if data is None or data == []:
+        return Response(status_code=204)
+
+    escaped_data = [html_escape_multi(note) for note in data]
+    return ORJSONResponseObjKeep({'success': True, 'data': escaped_data})
+
+
 @router.get('/v1/network/{run_id}')
 async def get_network(run_id, user: User = Depends(authenticate)):
     if run_id is None or not is_valid_uuid(run_id):
@@ -252,7 +276,9 @@ def old_v1_runs_endpoint():
 async def get_runs(uri: str | None = None, branch: str | None = None, machine_id: int | None = None, machine: str | None = None, filename: str | None = None, job_id: int | None = None, failed: bool | None = None, limit: int | None = 50, uri_mode = 'none', user: User = Depends(authenticate)):
 
     query = '''
-            SELECT r.id, r.name, r.uri, r.branch, r.created_at, r.invalid_run, r.filename, r.usage_scenario_variables, m.description, r.commit_hash, r.end_measurement, r.failed, r.machine_id
+            SELECT r.id, r.name, r.uri, r.branch, r.created_at,
+            (SELECT COUNT(id) FROM warnings as w WHERE w.run_id = r.id) as invalid_run,
+            r.filename, r.usage_scenario_variables, m.description, r.commit_hash, r.end_measurement, r.failed, r.machine_id
             FROM runs as r
             LEFT JOIN machines as m on r.machine_id = m.id
             WHERE
@@ -333,6 +359,12 @@ async def compare_in_repo(ids: str, force_mode:str | None = None, user: User = D
 
     comparison_details = get_comparison_details(user, ids, comparison_db_key)
 
+    # check if a run failed
+
+    if check_run_failed(user, ids) >= 1:
+        raise RequestValidationError('At least one run in your runs to compare failed. Comparsion for failed runs is not supported.')
+
+
     if not (phase_stats := get_phase_stats(user, ids)):
         return Response(status_code=204) # No-Content
 
diff --git a/docker/structure.sql b/docker/structure.sql
@@ -52,6 +52,7 @@ VALUES (
         "api": {
             "quotas": {},
             "routes": [
+                "/v1/warnings/{run_id}",
                 "/v1/insights",
                 "/v1/ci/insights",
                 "/v1/machines",
@@ -341,6 +342,19 @@ CREATE TRIGGER notes_moddatetime
     FOR EACH ROW
     EXECUTE PROCEDURE moddatetime (updated_at);
 
+CREATE TABLE warnings (
+    id SERIAL PRIMARY KEY,
+    run_id uuid REFERENCES runs(id) ON DELETE CASCADE ON UPDATE CASCADE,
+    message text,
+    created_at timestamp with time zone DEFAULT now(),
+    updated_at timestamp with time zone
+);
+CREATE INDEX "warnings_run_id" ON "warnings" USING HASH ("run_id");
+CREATE TRIGGER warnings_moddatetime
+    BEFORE UPDATE ON warnings
+    FOR EACH ROW
+    EXECUTE PROCEDURE moddatetime (updated_at);
+
 CREATE TABLE ci_measurements (
     id SERIAL PRIMARY KEY,
     energy_uj bigint,
diff --git a/frontend/compare.html b/frontend/compare.html
@@ -75,6 +75,16 @@ <h3>Run Data</h3>
             </div>
         </div><!-- end ui full-width-card card -->
 
+        <div id="run-warnings" class="ui icon message warning hidden">
+            <i class="info warning icon"></i>
+            <div class="content">
+                <div class="header">
+                    Warnings - At least one run contains the following warnings
+                </div>
+                <ul></ul>
+            </div>
+        </div>
+
         <div class="ui steps attached phases">
             <a class="active step" data-tab="[BASELINE]">
                 <div class="content">
diff --git a/frontend/js/compare.js b/frontend/js/compare.js
@@ -18,6 +18,32 @@ async function fetchDiff() {
 
 }
 
+const fetchWarningsForRuns = async (ids) => {
+    const warnings = [];
+    for (const id of ids) {
+        try {
+            const data = await makeAPICall('/v1/warnings/' + id);
+            if (data?.data) warnings.push(...data.data);
+        } catch (err) {
+            showNotification('Could not get warnings data from API', err);
+        }
+    }
+    return warnings;
+};
+
+const fillWarnings = (warnings) => {
+    if (!warnings || warnings.length === 0) return;
+    const warnings_texts = warnings.map(sub => sub[1]);
+    const unique_warnings = [...new Set(warnings_texts)];
+
+    const container = document.querySelector('#run-warnings');
+    const ul = container.querySelector('ul');
+    unique_warnings.forEach(w => {
+        ul.insertAdjacentHTML('beforeend', `<li>${w}</li>`);
+    });
+    container.classList.remove('hidden');
+};
+
 $(document).ready( (e) => {
     (async () => {
         const url_params = getURLParams();
@@ -43,6 +69,9 @@ $(document).ready( (e) => {
             return
         }
 
+        const warnings = await fetchWarningsForRuns(url_params['ids'].split(','));
+        fillWarnings(warnings);
+
         let comparison_identifiers = phase_stats_data.comparison_identifiers.map((el) => replaceRepoIcon(el));
         comparison_identifiers = comparison_identifiers.join(' vs. ')
         document.querySelector('#run-data-top').insertAdjacentHTML('beforeend', `<tr><td><strong>Comparison Type</strong></td><td>${phase_stats_data.comparison_case}</td></tr>`)
diff --git a/frontend/js/helpers/runs.js b/frontend/js/helpers/runs.js
@@ -190,7 +190,8 @@ const getRunsTable = async (el, url, include_uri=true, include_button=true, sear
                 if(row[11] == true) el = `${el} <span class="ui red horizontal label">Failed</span>`;
                 else if(row[10] == null) el = `${el} (in progress 🔥)`;
 
-                if(row[5] != null) el = `${el} <span class="ui yellow horizontal label" title="${row[5]}">invalidated</span>`;
+
+                if(row[5] != 0) el = `${el} <span class="ui yellow horizontal label" title="${row[5]}">Warnings</span>`;
 
                 return `<a href="/stats.html?id=${row[0]}" target="_blank">${el}</a>`
             },
diff --git a/frontend/js/stats.js b/frontend/js/stats.js
@@ -119,13 +119,11 @@ const fetchAndFillRunData = async (url_params) => {
         } else if(item == 'name' || item == 'filename' || item == 'branch') {
             document.querySelector('#run-data-top').insertAdjacentHTML('beforeend', `<tr><td><strong>${item}</strong></td><td>${run_data?.[item]}</td></tr>`)
         } else if(item == 'failed' && run_data?.[item] == true) {
-            document.querySelector('#run-data-top').insertAdjacentHTML('beforeend', `<tr><td><strong>Status</strong></td><td><span class="ui red horizontal label">This run has failed. Please see logs for details</span></td></tr>`)
+            document.querySelector('#run-failed').classList.remove('hidden');
         } else if(item == 'start_measurement' || item == 'end_measurement') {
             document.querySelector('#run-data-accordion').insertAdjacentHTML('beforeend', `<tr><td><strong>${item}</strong></td><td title="${run_data?.[item]}">${new Date(run_data?.[item] / 1e3)}</td></tr>`)
         } else if(item == 'created_at' ) {
             document.querySelector('#run-data-accordion').insertAdjacentHTML('beforeend', `<tr><td><strong>${item}</strong></td><td title="${run_data?.[item]}">${new Date(run_data?.[item])}</td></tr>`)
-        } else if(item == 'invalid_run' && run_data?.[item] != null) {
-            document.querySelector('#run-data-top').insertAdjacentHTML('beforeend', `<tr><td><strong>${item}</strong></td><td><span class="ui yellow horizontal label">${run_data?.[item]}</span></td></tr>`)
         } else if(item == 'gmt_hash') {
             document.querySelector('#run-data-accordion').insertAdjacentHTML('beforeend', `<tr><td><strong>${item}</strong></td><td><a href="https://github.com/green-coding-solutions/green-metrics-tool/commit/${run_data?.[item]}">${run_data?.[item]}</a></td></tr>`);
         } else if(item == 'uri') {
@@ -144,10 +142,7 @@ const fetchAndFillRunData = async (url_params) => {
 
     document.querySelector('#run-data-accordion').insertAdjacentHTML('beforeend', `<tr><td><strong>duration</strong></td><td title="${measurement_duration_in_s} seconds">${measurement_duration_display}</td></tr>`)
 
-    if (run_data.invalid_run) {
-        showNotification('Run measurement has been marked as invalid', run_data.invalid_run);
-        document.body.classList.add("invalidated-measurement")
-    }
+    // warnings will be fetched separately
 
 }
 
@@ -592,6 +587,25 @@ const fetchTimelineNotes = async (url_params) => {
     return notes?.data;
 }
 
+const fetchAndFillWarnings = async (url_params) => {
+    let warnings = null;
+    try {
+        warnings = await makeAPICall('/v1/warnings/' + url_params['id'])
+        if (!warnings || warnings?.data?.length === 0) return;
+    } catch (err) {
+        showNotification('Could not get warnings data from API', err);
+        return;
+    }
+
+    const container = document.querySelector('#run-warnings');
+    const ul = container.querySelector('ul');
+    warnings.data.forEach(w => {
+        ul.insertAdjacentHTML('beforeend', `<li>${w[1]}</li>`);
+    });
+    container.classList.remove('hidden');
+}
+
+
 
 /* Chart starting code*/
 $(document).ready( (e) => {
@@ -611,6 +625,7 @@ $(document).ready( (e) => {
         fetchAndFillNetworkIntercepts(url_params);
         fetchAndFillOptimizationsData(url_params);
         fetchAndFillAIData(url_params);
+        fetchAndFillWarnings(url_params);
 
         (async () => { // since we need to wait for fetchAndFillPhaseStatsData we wrap in async so later calls cann already proceed
             const phase_stats = await fetchAndFillPhaseStatsData(url_params);
diff --git a/frontend/stats.html b/frontend/stats.html
@@ -126,6 +126,26 @@ <h3>Usage Scenario File</h3>
             </div>
         </div><!-- end ui full-width-card card -->
 
+        <div id="run-failed" class="ui icon message error hidden">
+            <i class="times icon"></i>
+            <div class="content">
+                <div class="header">
+                    Failed
+                </div>
+                This run has failed. Please see logs for details
+            </div>
+        </div>
+
+        <div id="run-warnings" class="ui icon message warning hidden">
+            <i class="warning icon"></i>
+            <div class="content">
+                <div class="header">
+                    Warnings
+                </div>
+                <ul></ul>
+            </div>
+        </div>
+
         <div class="ui steps attached phases">
             <a class="active step" data-tab="[BASELINE]">
                 <div class="content">
diff --git a/lib/scenario_runner.py b/lib/scenario_runner.py
@@ -151,6 +151,7 @@ def __init__(self,
         self.__working_folder_rel = ''
         self.__image_sizes = {}
         self.__volume_sizes = {}
+        self.__warnings = []
 
         # we currently do not use this variable
         # self.__filename = self._original_filename # this can be changed later if working directory changes
@@ -226,7 +227,9 @@ def check_system(self, mode='start'):
             return
 
         if mode =='start':
-            system_checks.check_start()
+            warnings = system_checks.check_start()
+            for warn in warnings:
+                self.__warnings.append(warn)
         else:
             raise RuntimeError('Unknown mode for system check:', mode)
 
@@ -1281,6 +1284,13 @@ def add_to_log(self, container_name, message, cmd=''):
             self.__stdout_logs[log_entry_name] = ''
         self.__stdout_logs[log_entry_name] = '\n'.join((self.__stdout_logs[log_entry_name], message))
 
+    def save_warnings(self):
+        if not self._run_id or self._dev_no_save:
+            print("Skipping saving warning due to missing run id or --dev-no-save")
+            return
+        for message in self.__warnings:
+            DB().query("INSERT INTO warnings (run_id, message) VALUES (%s, %s)", (self._run_id, message))
+
     def add_containers_to_metric_providers(self):
         for metric_provider in self.__metric_providers:
             if metric_provider._metric_name.endswith('_container'):
@@ -1763,12 +1773,7 @@ def identify_invalid_run(self):
             if not self._run_id or self._dev_no_save:
                 print(TerminalColors.WARNING, '\nSkipping saving identification if run is invalid due to missing run id or --dev-no-save', TerminalColors.ENDC)
             else:
-                DB().query('''
-                    UPDATE runs
-                    SET invalid_run = COALESCE(invalid_run, '') || %s
-                    WHERE id=%s''',
-                    params=(invalid_message, self._run_id)
-                )
+                self.__warnings.append(invalid_message)
 
         for argument in self._arguments:
             # dev no optimizations does not make the run invalid ... all others do
@@ -1779,12 +1784,7 @@ def identify_invalid_run(self):
                 if not self._run_id or self._dev_no_save:
                     print(TerminalColors.WARNING, '\nSkipping saving identification if run is invalid due to missing run id or --dev-no-save', TerminalColors.ENDC)
                 else:
-                    DB().query('''
-                        UPDATE runs
-                        SET invalid_run = COALESCE(invalid_run, '') || %s
-                        WHERE id=%s''',
-                        params=(invalid_message, self._run_id)
-                    )
+                    self.__warnings.append(invalid_message)
                 break # one is enough
 
     def cleanup(self, continue_measurement=False):
@@ -1841,6 +1841,7 @@ def cleanup(self, continue_measurement=False):
         self.__working_folder_rel = ''
         self.__image_sizes = {}
         self.__volume_sizes = {}
+        self.__warnings = []
 
 
         print(TerminalColors.OKBLUE, '-Cleanup gracefully completed', TerminalColors.ENDC)
@@ -1993,23 +1994,30 @@ def run(self):
                                 raise exc
                             finally:
                                 try:
-                                    if self._run_id and self._dev_no_phase_stats is False and self._dev_no_save is False:
-                                        # After every run, even if it failed, we want to generate phase stats.
-                                        # They will not show the accurate data, but they are still neded to understand how
-                                        # much a failed run has accrued in total energy and carbon costs
-                                        print(TerminalColors.HEADER, '\nCalculating and storing phases data. This can take a couple of seconds ...', TerminalColors.ENDC)
-
-                                        # get all the metrics from the measurements table grouped by metric
-                                        # loop over them issuing separate queries to the DB
-                                        from tools.phase_stats import build_and_store_phase_stats # pylint: disable=import-outside-toplevel
-                                        build_and_store_phase_stats(self._run_id, self._sci)
-
+                                    self.save_warnings()
                                 except BaseException as exc:
                                     self.add_to_log(exc.__class__.__name__, str(exc))
                                     self.set_run_failed()
                                     raise exc
                                 finally:
-                                    self.cleanup()  # always run cleanup automatically after each run
+                                    try:
+                                        if self._run_id and self._dev_no_phase_stats is False and self._dev_no_save is False:
+                                            # After every run, even if it failed, we want to generate phase stats.
+                                            # They will not show the accurate data, but they are still neded to understand how
+                                            # much a failed run has accrued in total energy and carbon costs
+                                            print(TerminalColors.HEADER, '\nCalculating and storing phases data. This can take a couple of seconds ...', TerminalColors.ENDC)
+
+                                            # get all the metrics from the measurements table grouped by metric
+                                            # loop over them issuing separate queries to the DB
+                                            from tools.phase_stats import build_and_store_phase_stats # pylint: disable=import-outside-toplevel
+                                            build_and_store_phase_stats(self._run_id, self._sci)
+
+                                    except BaseException as exc:
+                                        self.add_to_log(exc.__class__.__name__, str(exc))
+                                        self.set_run_failed()
+                                        raise exc
+                                    finally:
+                                        self.cleanup()  # always run cleanup automatically after each run
 
 
 
diff --git a/lib/system_checks.py b/lib/system_checks.py
diff --git a/migrations/2025_07_01_warnings.sql b/migrations/2025_07_01_warnings.sql
diff --git a/tests/smoke_test.py b/tests/smoke_test.py
diff --git a/tests/test_runner.py b/tests/test_runner.py
