Skip to content
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

DQM: Remove ClientConfig package #28380

Merged
merged 12 commits into from Feb 10, 2020

Conversation

schneiml
Copy link
Contributor

@schneiml schneiml commented Nov 12, 2019

PR description:

[This PR is the last one including changes from #28316. It is based on #28379, which includes the changes that make this PR possible] Edit: rebased after these PRs are merged.

This PR removes the DQMServices/ClientConfig package. The more important parts of its functionality (DQMGenericClient, QualityTester) where moved to DQMServices/Components in #28379. What remains is DQMParserBase, some sort of XML parsing helper based on Xerces, and a few more random files.

The few modules that where using DQMParserBase (SiPixelMonitorClient/SiStripMonitorClient/SiStripCommissioningClients) now have their parsers stubbed out. The only one of them used in the (short matrix) tests seems to be one part of SiPixelMonitorClient, which has been replaced with a boost::property_tree based parser. I also implemented a parser for SiStripMonitorClient, but could not actually test it yet (it may be used in online DQM, which I can test at some point). The others remain stubbed out, and while I could easily replace them, I am hesitant to do so without being able to test them at all.

PR validation:

None for this PR, but some in #28316. No changes in PR tests expected, but most of the affected code is not covered by the PR tests. The stubbed out implementations clearly won't work, but it is questionable if they are still used anywhere. The assertions should make sure that anything using the missing functionality crashes.

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28380/12710

  • This PR adds an extra 684KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @schneiml (Marcel Schneider) for master.

It involves the following packages:

Alignment/OfflineValidation
CalibMuon/DTCalibration
Calibration/EcalCalibAlgos
Calibration/TkAlCaRecoProducers
DQM/BeamMonitor
DQM/CSCMonitorModule
DQM/DTMonitorClient
DQM/DTMonitorModule
DQM/EcalCommon
DQM/EcalMonitorDbModule
DQM/EcalMonitorTasks
DQM/EcalPreshowerMonitorClient
DQM/EcalPreshowerMonitorModule
DQM/HLTEvF
DQM/HLXMonitor
DQM/HcalTasks
DQM/Integration
DQM/L1TMonitor
DQM/L1TMonitorClient
DQM/Physics
DQM/RPCMonitorClient
DQM/SiPixelCommon
DQM/SiPixelMonitorClient
DQM/SiPixelPhase1Config
DQM/SiPixelPhase1Summary
DQM/SiStripCommissioningClients
DQM/SiStripCommissioningSources
DQM/SiStripMonitorClient
DQM/SiStripMonitorSummary
DQM/TrackingMonitor
DQM/TrackingMonitorClient
DQMOffline/CalibMuon
DQMOffline/EGamma
DQMOffline/Ecal
DQMOffline/JetMET
DQMOffline/L1Trigger
DQMOffline/Muon
DQMOffline/PFTau
DQMOffline/Trigger
DQMServices/ClientConfig
DQMServices/Components
DQMServices/Core
DQMServices/Examples
GeneratorInterface/RivetInterface
HLTrigger/Timer
HLTriggerOffline/B2G
HLTriggerOffline/Common
HLTriggerOffline/Exotica
HLTriggerOffline/Higgs
HLTriggerOffline/JetMET
HLTriggerOffline/Muon
HLTriggerOffline/SUSYBSM
HLTriggerOffline/Tau
PhysicsTools/NanoAOD
Validation/L1T
Validation/RecoB
Validation/RecoMuon
Validation/RecoTau
Validation/RecoTrack

@SiewYan, @andrius-k, @schneiml, @Martin-Grunewald, @rekovic, @fioriNTU, @tlampen, @alberto-sanchez, @pohsun, @santocch, @peruzzim, @cmsbuild, @agrohsje, @fwyzard, @efeyazgan, @tocheng, @jfernan2, @qliphy, @benkrikler, @mkirsano, @kmaeshima, @christopheralanwest, @franzoni, @fgolf can you please review it and eventually sign? Thanks.
@erikbutz, @rappoccio, @felicepantaleo, @schoef, @emilbols, @missirol, @argiro, @HeinerTholen, @fioriNTU, @tlampen, @alberto-sanchez, @ahinzmann, @threus, @mmusich, @seemasharmafnal, @venturia, @acimmino, @mmarionncern, @kreczko, @hdelanno, @calderona, @makortel, @smoortga, @acaudron, @jhgoh, @jdolen, @HuguesBrun, @agrohsje, @cericeci, @ferencek, @trocino, @rociovilar, @abbiendi, @barvic, @DryRun, @GiacomoSguazzoni, @tocheng, @VinInn, @jdamgov, @bellan, @nhanvtran, @gkasieczka, @rovere, @jandrea, @mschrode, @idebruyn, @ebrondol, @ptcox, @mtosi, @dgulhan, @clelange, @mkirsano, @Martin-Grunewald, @batinkov, @adewit, @battibass, @Fedespring, @JyothsnaKomaragiri, @mverzett, @wmtford, @gpetruc, @mariadalfonso, @pvmulder, @andrzejnovak, @folguera this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 5, 2020

+1
Tested at: 68beb45
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4a4887/4511/summary.html
CMSSW: CMSSW_11_1_X_2020-02-05-1100
SCRAM_ARCH: slc7_amd64_gcc820

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 5, 2020

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 5, 2020

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4a4887/4511/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 34
  • DQMHistoTests: Total histograms compared: 2698043
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2697696
  • DQMHistoTests: Total skipped: 346
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 33 files compared)
  • Checked 147 log files, 16 edm output root files, 34 DQM output files

@schneiml
Copy link
Contributor Author

schneiml commented Feb 6, 2020

+1

assuming this is fine from Tracker side now (else, @rgerosa quickly jump in).

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 6, 2020

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. @davidlange6, @silviodonato, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

@mmusich
Copy link
Contributor

mmusich commented Feb 6, 2020

I also implemented a parser for SiStripMonitorClient, but could not actually test it yet (it may be used in online DQM, which I can test at some point). The others remain stubbed out, and while I could easily replace them, I am hesitant to do so without being able to test them at all.

@arossi83 @sroychow please comment if there is any remaining concern and / or if you can provide suitable test configurations.

@silviodonato
Copy link
Contributor

hold
waiting for feedback from @arossi83 @sroychow about the test of the parser of SiStripMonitorClient

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 6, 2020

Pull request has been put on hold by @silviodonato
They need to issue an unhold command to remove the hold state or L1 can unhold it for all

@sroychow
Copy link
Contributor

sroychow commented Feb 7, 2020

@silviodonato Tracker DQM is okay with this PR to be merged. Few bugs were discovered in configuration files in DQM/SiStripMonitorClient package. We shall make a separate PR for those.

@silviodonato
Copy link
Contributor

unhold

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 9, 2020

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. @davidlange6, @silviodonato, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

@silviodonato
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 2d56239 into cms-sw:master Feb 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants