search

cms-sw / cmssw

CMS Offline Software
http://cms-sw.github.io/
Apache License 2.0
1.08k stars 4.3k forks source link

remove ESHandle from pps sampic pcl #38960

Open ChrisMisan opened 2 years ago

ChrisMisan commented 2 years ago

By the way (not changed in this PR so for longer term), the PPSDiamondSampicTimingCalibrationPCLHarvester gets an EventSetup product in its beginRun() and stores it in a member ESHandle https://github.com/cms-sw/cmssw/blob/dbcf1d6e9834a91def96098fb13570a2a0d88098/CalibPPS/TimingCalibration/plugins/PPSDiamondSampicTimingCalibrationPCLHarvester.cc#L82-L84 and then uses it in dqmEndJob() (which is really endProcessBlock() transition) https://github.com/cms-sw/cmssw/blob/dbcf1d6e9834a91def96098fb13570a2a0d88098/CalibPPS/TimingCalibration/plugins/PPSDiamondSampicTimingCalibrationPCLHarvester.cc#L187-L188 https://github.com/cms-sw/cmssw/blob/dbcf1d6e9834a91def96098fb13570a2a0d88098/CalibPPS/TimingCalibration/plugins/PPSDiamondSampicTimingCalibrationPCLHarvester.cc#L265-L269

While this pattern may work in practice today, it is not guaranteed to work in the future. Objects owned by the EventSetup system should be used only in transition functions that take the edm::EventSetup argument. A workaround would be to copy the relevant products instead of passing an ESHandle or a raw pointer.

Originally posted by @makortel in https://github.com/cms-sw/cmssw/issues/38893#issuecomment-1201394267

cmsbuild commented 2 years ago

A new Issue was created by @ChrisMisan Christopher.

@Dr15Jones, @perrotta, @dpiparo, @rappoccio, @makortel, @smuzaffar, @qliphy can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

makortel commented 2 years ago

assign alca

cmsbuild commented 2 years ago

New categories assigned: alca

@yuanchao,@francescobrivio,@malbouis,@saumyaphor4252,@tvami,@ChrisMisan you have been requested to review this Pull request/Issue and eventually sign? Thanks