Add possibility to check if a cached object is valid

[njacazio]

No description provided.

[shahor02]

Hi @njacazio

Just noticed these validity ranges added to cached objects. Note that in general the limits we extract from the header is not a real validity since they don't account for the overlaps with other detectors.

[njacazio]

Hi @shahor02 I see, thanks!
However in analysis it should probably be fine as the use case is more simple.
This is a feature on which we rely for the PID response implementation as we want to avoid fetching the same object more than once, if the object are created with correct validity on the ccdb it should probably be fine, right?

[shahor02]

Hi @njacazio
It is probably ok for your use case, but not in general where there could be an overlaps. I would suggest introducing a special flag which forces the validity check based only on the limits of the cached object but the default behaviour should be full check with the query passing the ETag of the cached object.

[njacazio]

Hi @shahor02
I looked a bit more in the code and it seems to me that this is already the case i.e. the etag is used by default and one can ask instead to use the timestamp validity of the object, provided that the scheme on the ccdb ensures no overlap.
One can set this behavior with setLocalObjectValidityChecking, in this case the object will not be retrieved anymore and instead its validity will be checked before, but indeed the method isCachedObjectValid based on the timestamp (the one added with this PR) might be confusing and lead to issues in the future. What do you suggest? We can revert this PR if needed

[jgrosseo]

Is this only for analysis? Then I also think that setLocalObjectValidityChecking was already solving it and we should revert...

[njacazio]

I agree, I will update the other PR

[shahor02]

OK, indeed, running w/o setLocalObjectValidityChecking(true) should be safe, but I see that

it grep setLocalObjectValidityChecking
CCDB/README.md:In cached mode, the manager can check that local objects are still valid by requiring `mgr.setLocalObjectValidityChecking(true)`, in this case a CCDB query is performed only if the cached object is no longer valid.
CCDB/include/CCDB/BasicCCDBManager.h:  void setLocalObjectValidityChecking(bool v = true) { mCheckObjValidityEnabled = v; }
Detectors/CPV/simulation/src/Digitizer.cxx:    ccdbMgr.setLocalObjectValidityChecking(true); //query objects from remote site only when local one is not valid
Detectors/CPV/simulation/src/RawWriter.cxx:    ccdbMgr.setLocalObjectValidityChecking(true); // query objects from remote site only when local one is not valid
Steer/DigitizerWorkflow/src/SimpleDigitizerWorkflow.cxx:    o2::ccdb::BasicCCDBManager::instance().setLocalObjectValidityChecking(true);

@sawenzel what was the reason to add this in the SimpleDigitizerWorkflow ? (I guess CPV simply copy/pasted it).
I understand that digitziation is usually done over single TF, but then why it is needed at all?

[njacazio]

Btw, this PR can now be reverted as the feature it introduced is no longer needed in AliceO2Group/O2Physics#874 thanks!

[njacazio]

Hi @jgrosseo was this reverted at the end? @shahor02 indeed I see that the setLocalObjectValidityChecking is still enabled, as you said:

Detectors/CPV/simulation/src/Digitizer.cxx:    ccdbMgr.setLocalObjectValidityChecking(true); //query objects from remote site only when local one is not valid
Detectors/CPV/simulation/src/RawWriter.cxx:    ccdbMgr.setLocalObjectValidityChecking(true); // query objects from remote site only when local one is not valid
CCDB/README.md:In cached mode, the manager can check that local objects are still valid by requiring `mgr.setLocalObjectValidityChecking(true)`, in this case a CCDB query is performed only if the cached object is no longer valid.
CCDB/include/CCDB/BasicCCDBManager.h:  void setLocalObjectValidityChecking(bool v = true) { mCheckObjValidityEnabled = v; }
Steer/DigitizerWorkflow/src/SimpleDigitizerWorkflow.cxx:    o2::ccdb::BasicCCDBManager::instance().setLocalObjectValidityChecking(true);

Is this still intended? Shall we also update the README to be explicit about this option?