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
GT Initialization: ignoring non-existing record in the plugins pre-load #33798
GT Initialization: ignoring non-existing record in the plugins pre-load #33798
Conversation
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33798/22771
Code check has found code style and quality issues which could be resolved by applying following patch(s)
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33798/22773
|
A new Pull Request was created by @ggovi for master. It involves the following packages: CondCore/ESSources @malbouis, @yuanchao, @cmsbuild, @tlampen, @ggovi, @pohsun, @francescobrivio can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-dd3d9b/15211/summary.html Comparison SummarySummary:
|
+1 |
Removed from LogWarning message unnecessary newline Co-authored-by: Matti Kortelainen <matti.kortelainen@cern.ch>
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33798/22864
|
please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-dd3d9b/15310/summary.html Comparison SummarySummary:
|
+1 |
+alca |
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. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2) |
@fwyzard asked by e-mail what happens if a record class is changed (from v1 to v2). Shortly, in both cases, there is no risk to read a payload in a wrong way. @ggovi have I understood it correctly? |
The record class in itself cannot change, as its scope is essentially labelling a data consumption context. In theory, what can be changed is the CondFormat class associated via the macro 'REGISTER_PLUGIN'. This change is already forbidden by policy. Anyhow, non-spotted changes of this type cannot cause mis-interpreted reading of stored payload, as the de-serialization is attempted only when the payload class matches the record class. In case of mis-match, an exception is thown at the payload consuming time. This behavior is not altered by the present PR. |
+1 |
PR description:
CMSSW Workflows requiring the consumption of conditions can be successfully executed when all the records involved in the code are represented by their corresponding entries in the Global Tag loaded from the Condition Database.
Conversely, additional Records included in the GT Record-Tag map could be ignored when they are not involved in the Workflow code. This is the case, for instance, of Records that have been completely removed from the CMSSW Code.
This PR is addressing this last particular circumstance. The GT initialisation has been modify to ignore the Records for which the corresponding plugin can't be found in CMSSW.
The new behaviour is obviously applied to Records added manually with toGet statements, when not present in the release.
Note that these changes are not altering the current behaviour for other possible mis-matches between release and database, that could happen at later stage in the processing:
PR validation:
Integration test, executed with a specific GT (106X_mcRun2_asymptotic_v15) triggering in 12X the condition of 'missing record plugin'