Skip to content

Configure CCDB server via NameConf::mCCDBServer and NameConf::getCCDBServer()#7778

Merged
davidrohr merged 2 commits intoAliceO2Group:devfrom
shahor02:pr_fit_ccdb
Dec 3, 2021
Merged

Configure CCDB server via NameConf::mCCDBServer and NameConf::getCCDBServer()#7778
davidrohr merged 2 commits intoAliceO2Group:devfrom
shahor02:pr_fit_ccdb

Conversation

@shahor02
Copy link
Copy Markdown
Collaborator

@shahor02 shahor02 commented Dec 1, 2021

No description provided.

AllaMaevskaya
AllaMaevskaya previously approved these changes Dec 1, 2021
@shahor02 shahor02 changed the title Use alice-ccdb.cern.ch insted of o2-ccdb.internal as default for FIT [WIP] Use alice-ccdb.cern.ch insted of o2-ccdb.internal as default for FIT Dec 1, 2021
@shahor02 shahor02 changed the title [WIP] Use alice-ccdb.cern.ch insted of o2-ccdb.internal as default for FIT Configure CCDB server via NameConf::mCCDBServer and NameConf::getCCDBServer() Dec 1, 2021
Copy link
Copy Markdown
Collaborator

@noferini noferini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For TOF it is fine,
since I have an open PR affecting calibration and ccdb I will take care to align the TOF code also there once this is in dev

Copy link
Copy Markdown
Contributor

@martenole martenole left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @shahor02
from TRD side looks good. I was wondering if it would make sense to change the default URL for the CCDB manager as well? If I do

root [0] auto &mgr = o2::ccdb::BasicCCDBManager::instance()
[INFO] Is alien token present?: 0
(o2::ccdb::BasicCCDBManager &) @0x7f0c8ccb3028
root [1] mgr.getURL()
(const std::string &) "http://ccdb-test.cern.ch:8080"
root [2] 

I still get the test CCDB. And for instance the tracklet transformer is not setting the URL explicitly:

auto& ccdbmgr = o2::ccdb::BasicCCDBManager::instance();
ccdbmgr.setTimestamp(timestamp);
mCalibration = ccdbmgr.get<o2::trd::CalVdriftExB>("TRD/Calib/CalVdriftExB");

I would change this now, but I don't know if this is the case also in other places of O2..

Copy link
Copy Markdown
Collaborator

@davidrohr davidrohr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thx a lot for taking care!
Full CI is failing, I am just trying it locally.

@davidrohr
Copy link
Copy Markdown
Collaborator

Hi @shahor02
from TRD side looks good. I was wondering if it would make sense to change the default URL for the CCDB manager as well? If I do

Absolutely, I would do that. This configKeyValues should set all CCDB defaults.

@shahor02
Copy link
Copy Markdown
Collaborator Author

shahor02 commented Dec 2, 2021

Full CI is failing, I am just trying it locally.
This is because some ccdb objects, particularly, that of ZDC, exist only on the test ccdb server.

@shahor02
Copy link
Copy Markdown
Collaborator Author

shahor02 commented Dec 2, 2021

Hi @shahor02
from TRD side looks good. I was wondering if it would make sense to change the default URL for the CCDB manager as well? If I do

Absolutely, I would do that. This configKeyValues should set all CCDB defaults.

This would mean making CCDB dependent on the DataFormats. So far, we were trying to keep CCDB clean, the only O2 dependency is on O2::CommonUtils (which contains the ConfigurableParam class). What we can do is to define a special configurable CCDBParam and move the NameConf::mCCDBServer there.

@davidrohr
Copy link
Copy Markdown
Collaborator

I don't see a large problem if the CCDB depended on data formats.
Although, perhaps I would move the nameConf from DataFormats/Detectors/Common to DataFormats/common.

Having another configKeyValues class for the CCDB would also be fine for me.
Don't have a strong preference.

@shahor02
Copy link
Copy Markdown
Collaborator Author

shahor02 commented Dec 2, 2021

@cortesep I've copied the last objects in ZDC/Config/Sim and ZDC/Config/Module from http://ccdb-test.cern.ch:8080 to http://alice-ccdb.cern.ch. If you need to update them, please do this directly on the production server.

@shahor02
Copy link
Copy Markdown
Collaborator Author

shahor02 commented Dec 2, 2021

Although, perhaps I would move the nameConf from DataFormats/Detectors/Common to DataFormats/common.

NameConf depends on DetID which is in the DataFormats/Detectors/Common and which in turn depends on the DataFormats/common. We cannot move NameConf there w/o major refactoring...

@davidrohr
Copy link
Copy Markdown
Collaborator

davidrohr commented Dec 2, 2021

I am still getting errors, also for ZDC (ran 1 minute ago), these objects are missing:

Requested resource does not exist: http://alice-ccdb.cern.ch//ZDC/Config/Module/0/
Requested resource does not exist: http://alice-ccdb.cern.ch//CTP/Config/1638438988548/
Requested resource does not exist: http://alice-ccdb.cern.ch//CPV/Calib/Gains/1638438995255/
Requested resource does not exist: http://alice-ccdb.cern.ch/download/dcbe56d0-3aa1-11ec-9661-0aa202a21b9a
Requested resource does not exist: http://alice-ccdb.cern.ch/download/61a99a90-3aa6-11ec-9661-0aa202a21b9a

I don't fully understand what is the last one? It is requested by CPV.

@davidrohr
Copy link
Copy Markdown
Collaborator

Although, perhaps I would move the nameConf from DataFormats/Detectors/Common to DataFormats/common.

NameConf depends on DetID which is in the DataFormats/Detectors/Common and which in turn depends on the DataFormats/common. We cannot move NameConf there w/o major refactoring...

Well, it is only used only in some helper functions to get the detector name. Let me see whether I can find a clever way to move it to DataFormats/common without much refactoring.

@davidrohr
Copy link
Copy Markdown
Collaborator

Here is an attempt to move NameConf: davidrohr@8a59520
I had to move it to CommonUtils, for StringUtils and ConfigurableParam.
Not fully working yet, will fix the remaining issues later.

@davidrohr
Copy link
Copy Markdown
Collaborator

I have opened #7798, which includes this one.
I wanted to open the PR against shahor02:pr_fit_ccdb but that doesn't work since pr_fit_ccdb is not on dev, and meanwhile there were commits in dev that needed adaptation after I have moved the NameConf class to CommonUtils.

@davidrohr davidrohr merged commit cf26a9c into AliceO2Group:dev Dec 3, 2021
@shahor02 shahor02 deleted the pr_fit_ccdb branch January 29, 2022 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

6 participants