Fixing trigger results in el9 [issue #2795] - #2797
Conversation
compareTriggerResults.py and compareTriggerResultsSummary.py used the
old-style "#!/usr/bin/env python" shebang and were invoked directly in
run-pr-comparisons, bypassing the ${CMSBOT_PYTHON_CMD} fallback used by
every other Python call in that script. On el9 (CMSSW_20_1_X) there is
no bare "python" binary, so the scripts failed to even start
(env: 'python': No such file or directory), and the resulting silent
failure plus "|| true" meant the entire TriggerResults section
disappeared from the PR bot comment with no visible error.
Fixes cms-sw#2795.
Co-authored-by: artlbv <arturlbv@gmail.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QDeG9GoVo6Qzf9kjHm71nf
Fix silently-vanishing edm::TriggerResults comparisons on el9
|
A new Pull Request was created by @artlbv for branch master. @akritkbehera, @cmsbuild, @iarspider, @raoatifshad, @smuzaffar can you please review it and eventually sign? Thanks. |
|
cms-bot internal usage |
|
please test thanks @artlbv for opening this PR. We just need to make sure that this does not break PR tests for old release cycles 5.3., 8.0 etc. |
|
please test for CMSSW_8_0_X |
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-2ba0ec/54214/summary.html Comparison SummarySummary:
|
|
could you also test for 17 and 20X? |
|
please test for CMSSW_17_0_X |
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-2ba0ec/54213/summary.html Comparison SummaryThere are some workflows for which there are errors in the baseline: Summary:
|
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-2ba0ec/54219/summary.html Comparison SummarySummary:
|
No, it doesn't look like a useful output. I reproduced the issue by running the script locally. As the script is moved to diff --git a/compareTriggerResults.py b/compareTriggerResults.py
index c2eb16b..502ddbe 100644
--- a/compareTriggerResults.py
+++ b/compareTriggerResults.py
@@ -20,7 +20,7 @@ def WARNING(message):
def get_output(cmds, permissive=False):
- prc = subprocess.Popen(cmds, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+ prc = subprocess.Popen(cmds, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True)
out, err = prc.communicate()
if (not permissive) and prc.returncode:
KILL(Otherwise, this call produces an output that gets misinterpreted by the script, leading to the warning you spotted. In my simple local test, this seems to be enough. |
|
Pull request #2797 was updated. |
|
please test |
|
please test for CMSSW_17_0_X |
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-2ba0ec/54234/summary.html Comparison SummarySummary:
|
|
please test for CMSSW_8_0_X |
|
https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_20_1_X_2026-06-23-2300+2ba0ec/76347/triggerResults/ looks good now for 20.1.X ( with python3 ) |
|
Thanks, Shahzad. Yes, looks okay. Out of curiosity, what's the policy regarding the error reported in one of the GitHub actions (probably a python code-format error)? Should it be fixed here, or not? 
|
I will fix this once PR tests are passed |
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-2ba0ec/54235/summary.html Comparison SummarySummary:
|
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-2ba0ec/54241/summary.html Comparison SummarySummary:
|
|
+externals |
cmsbuild
added
fully-signed
externals-approved
and removed
pending-signatures
externals-pending
labels
|
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @mandrenguyen, @ftenchini, @sextonkennedy (and backports should be raised in the release meeting by the corresponding L2) |
5 participants
Summary
compareTriggerResults.py/compareTriggerResultsSummary.pyto use#!/usr/bin/env python3instead of the old#!/usr/bin/env pythonshebang,and wrap both invocations in
pr_testing/run-pr-comparisonswith${CMSBOT_PYTHON_CMD}, matching every other Python call in that script.pythonbinary, so these twoscripts previously failed immediately with
env: 'python': No such file or directory, and the failure was swallowedsilently (
|| true), causing the entireedm::TriggerResultscomparisonsection to disappear from PR bot comments with no visible error.
SUMMARY TriggerResults: WARNING ...print on the existing"zero inputs" early-return path in both scripts, so any future cause of
"comparison produced nothing" is surfaced in the PR comment instead of
vanishing silently.
Fixes #2795.
Test plan
python3 -m py_compile compareTriggerResults.py compareTriggerResultsSummary.pybash -n pr_testing/run-pr-comparisonscompareTriggerResultsSummary.pyagainst an empty input directory and confirmed the newSUMMARY TriggerResults: WARNING ...line is printededm::TriggerResultscomparisons reappear in a real CMSSW_20_1_X (el9) PR-bot comment