-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix LHCInfoPerLS PopCon incorrectly matching LS between OMS and PPS db #42629
Fix LHCInfoPerLS PopCon incorrectly matching LS between OMS and PPS db #42629
Conversation
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42629/36685
|
A new Pull Request was created by @JanChyczynski (jan_chyczynski) for master. It involves the following packages:
@perrotta, @consuegs, @cmsbuild, @saumyaphor4252, @francescobrivio, @tvami can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
please test |
-1 Failed Tests: UnitTests Unit TestsI found errors in the following unit tests: ---> test CondToolsLHCInfoNewPopConTest had ERRORS Comparison SummarySummary:
|
370cb85
to
48a34fc
Compare
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42629/36717
|
Pull request #42629 was updated. @perrotta, @consuegs, @cmsbuild, @saumyaphor4252, @francescobrivio, @tvami can you please check and sign again. |
I've run the unit test not realizing I didn't build the code after the last changes so I thought it was passing - stupid mistake. I've fixed it now. |
please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-eec0ec/34473/summary.html Comparison SummarySummary:
|
48a34fc
to
dbe2cd1
Compare
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42629/36764
|
Pull request #42629 was updated. @perrotta, @consuegs, @cmsbuild, @saumyaphor4252, @francescobrivio, @tvami can you please check and sign again. |
please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-eec0ec/34516/summary.html Comparison SummarySummary:
|
+db |
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. @perrotta, @dpiparo, @antoniovilela, @rappoccio (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
PR description:
Fixes a bug in
LHCInfoPerLS
PopCon: before it was writing a payload on a wrong IOV (corresponding to a different lumisection) or skipping a payload in these cases:start_time
of lumisection in OMS greater or equal toDIP_UPDATE_TIME
in PPS DBAdditionally it introduces logs monitoring payload quality to
LHCInfoPerLS
PopCon. It logs payloads with invalid values of beta star and crossing angle as well as missing records in the PPS DB.PR validation:
Added validation of writing correct number of payloads for fill 7967 to the
CondToolsLHCInfoNewPopConTest
unit test. Before the fix payload on IOV 7119284725528657920 was skipped.Backporting
I suspect it should be backproted back to 13_0_X but please confirm it.