Review: Allow defining valid range of Energy values for LHCInfoPerFill O2O

Author: JanChyczynski

CondTools/RunInfo/interface/TestLHCInfoPerFillPopConSourceHandler.h

@@ -10,6 +10,11 @@
 #include <string>
 #include <tuple>
 
+/*
+* A mock PopCon source handler for testing LHCInfoPerFillPopCon logic without
+* external dependencies (e.g. OMS, subsystem DBs, destination DB).
+*/
+
 class TestLHCInfoPerFillPopConSourceHandler : public LHCInfoPerFillPopConSourceHandler {
 public:
   TestLHCInfoPerFillPopConSourceHandler(edm::ParameterSet const& pset);
@@ -19,7 +24,7 @@ class TestLHCInfoPerFillPopConSourceHandler : public LHCInfoPerFillPopConSourceH
   std::map<unsigned short /*fillNr*/, cond::OMSServiceResult> mockLumiData;
   boost::posix_time::ptime mockExecutionTime;
 
-  Container& iovs() { return m_iovs; }
+  const Container& iovs() const { return m_iovs; }
 
 protected:
   std::unique_ptr<LHCInfoPerFill> findFillToProcess(cond::OMSService& oms,

CondTools/RunInfo/python/LHCInfoPerFillPopConAnalyzer_cfg.py

@@ -91,8 +91,7 @@
                 , """duringFill only: run-unique key for writing with OnlinePopCon
                      (used for confirming proper upload)"""
                   )
-
-
+# checking for invalid energy values, duringFill mode specific
 options.register('minEnergy',  450  
                 , VarParsing.VarParsing.multiplicity.singleton, VarParsing.VarParsing.varType.float
                 , """duringFill only: [GeV] min value of the range of valid values (inclusive).
@@ -101,7 +100,7 @@
                 , VarParsing.VarParsing.multiplicity.singleton, VarParsing.VarParsing.varType.float
                 , """duringFill only: [GeV] max value of the range of valid values (inclusive).
                      If the value is outside of this range the payload is not uploaded""")
-options.register('throwOnInvalid', False,
+options.register('throwOnInvalid', False, # Intended production setup: False for endFill, True for duringFill
                 VarParsing.VarParsing.multiplicity.singleton,
                 VarParsing.VarParsing.varType.bool,
                 "duringFill only: If true, throw on invalid payloads; if false, filter them out.")
@@ -119,6 +118,8 @@
   raise ValueError("mode argument not provided. Supported modes are: duringFill endFill")
 if options.mode not in ("duringFill", "endFill"):
   raise ValueError("Wrong mode argument. Supported modes are: duringFill endFill")
+if options.throwOnInvalid and options.mode != "duringFill":
+  raise ValueError("throwOnInvalid option can be True only in duringFill mode")
 
 CondDBConnection = CondDB.clone( connect = cms.string( options.destinationConnection ) )
 CondDBConnection.DBParameters.messageLevel = cms.untracked.int32( options.messageLevel )

CondTools/RunInfo/python/LHCInfoPerLSPopConAnalyzer_cfg.py

@@ -131,8 +131,7 @@
                      The default payload is inserted after the last processed fill has ended
                      and there's no ongoing stable beam yet. """
                   )
-
-
+# checking for invalid values, duringFill mode specific
 # it's unlikely to ever use values different from the defaults, added as a parameter just in case
 options.register('minBetaStar',  0.1 
                 , VarParsing.VarParsing.multiplicity.singleton, VarParsing.VarParsing.varType.float
@@ -150,7 +149,7 @@
                 , VarParsing.VarParsing.multiplicity.singleton, VarParsing.VarParsing.varType.float
                 , """duringFill only: [urad] max value of the range of valid values (inclusive).
                      If the value is outside of this range the payload is not uploaded""")
-options.register('throwOnInvalid', False,
+options.register('throwOnInvalid', False, # Intended production setup: False for endFill, True for duringFill
                 VarParsing.VarParsing.multiplicity.singleton,
                 VarParsing.VarParsing.varType.bool,
                 "duringFill only: If true, throw on invalid payloads; if false, filter them out.")

CondTools/RunInfo/src/LHCInfoPerFillPopConSourceHandler.cc

@@ -520,12 +520,8 @@ LHCInfoPerFillPopConSourceHandler::executeLumiQuery(const cond::OMSService& oms,
   query->filterEQ("beams_stable", "true");
   query->limit(cond::lhcInfoHelper::kLumisectionsQueryLimit);
 
-  bool executed = query->execute();
-  if (executed) {
-    return std::make_tuple(query->result(), true, std::move(query));
-  } else {
-    return std::make_tuple(cond::OMSServiceResult(), false, std::move(query));
-  }
+  bool success = query->execute();
+  return {(success ? query->result() : cond::OMSServiceResult()), success, std::move(query)};
 }
 
 void LHCInfoPerFillPopConSourceHandler::getLumiData(const cond::OMSService& oms,

CondTools/RunInfo/test/BuildFile.xml

@@ -6,9 +6,8 @@
 <test name="CondToolsRunInfoTest" command="test_runinfo.sh"/>
 <test name="CondToolsLHCInfoTest" command="test_lhcInfo.sh"/>
 <ifarch value="x86_64">
-<test name="CondToolsLHCInfoNewPopConTest" command="test_lhcInfoNewPopCon.sh"/>
+  <test name="CondToolsLHCInfoNewPopConTest" command="test_lhcInfoNewPopCon.sh"/>
 </ifarch>
-
 <bin file="test_catch2_LHCInfoPerFillPopCon.cpp" name="TestLHCInfoPerFillPopCon">
   <use name="catch2"/>
   <use name="FWCore/ParameterSet"/>

CondTools/RunInfo/test/test_catch2_LHCInfoPerFillPopCon.cpp

@@ -342,7 +342,7 @@ TEST_CASE("LHCInfoPerFillPopConSourceHandler.getNewObjects fills IOVs correctly
       boost::posix_time::time_from_string("2023-06-01 12:00:00")};
   std::vector<boost::posix_time::ptime> fillEndTimes = {boost::posix_time::time_from_string("2023-06-01 12:30:00")};
 
-  for (size_t i = 0; i < 3; ++i) {
+  for (size_t i = 0; i < fillNumbers.size(); ++i) {
     auto fillPayload = std::make_shared<LHCInfoPerFill>();
     fillPayload->setFillNumber(fillNumbers[i]);
     fillPayload->setEnergy(energies[i]);