Ctpdev0#1012
Conversation
Barthelemy
left a comment
There was a problem hiding this comment.
Thank you for the new ctp module. Could you review my comments and make the changes ? Then I will re-review and approve.
| "url": "infologger:///debug?qc" | ||
| }, | ||
| "consul": { | ||
| "url": "http://consul-test.cern.ch:8500" |
There was a problem hiding this comment.
Please remove the address of consul.
| "url": "http://consul-test.cern.ch:8500" | |
| "url": "" |
| "id": "tst-raw", | ||
| "active": "true", | ||
| "machines": [], | ||
| "query": "random:TST/RAWDATA/0", |
There was a problem hiding this comment.
Are you sending your data under TST/RAWDATA ? shouldn't it be under something like CTP/RAWDATA ?
There was a problem hiding this comment.
I don't sample data (yet) so I left it as it was from example. I am changing tst to ctp in both cases.
There was a problem hiding this comment.
but if you are not sampling, what data are you monitoring ?
There was a problem hiding this comment.
This version is reading raw files locally - i.e. raw file is created from simulation (then digits, then raw file). I wanted to have it while waiting for interface by Sylvain. The payload structure will be the same in the real data so it is not waste of time. If I correctly understand, the data sampling is needed when you want monitor only fraction of data? here I monitor all (they are from simulation).
| @@ -0,0 +1,85 @@ | |||
| // Copyright 2019-2020 CERN and copyright holders of ALICE O2. | |||
There was a problem hiding this comment.
It seems that RawDataQcCheck is the default skeleton class. Please remove it.
There was a problem hiding this comment.
Yes it is. Do you mean removing RawDataQcCheck.cxx and RawDataQcCheck.h ?
There was a problem hiding this comment.
yes, removing the whole class if you don't use it .
There was a problem hiding this comment.
also to remove the check in .json file?
There was a problem hiding this comment.
and line in LinkDef.h?
|
|
||
| /// \brief Example Quality Control DPL Task | ||
| /// \author My Name | ||
| class RawDataQcTask final : public TaskInterface |
There was a problem hiding this comment.
Please name the class in a more meaningful way.
There was a problem hiding this comment.
Would CTPRawDataReaderTask be better?
| ILOG(Info, Support) << "initialize RawDataQcTask" << ENDM; // QcInfoLogger is used. FairMQ logs will go to there as well. | ||
|
|
||
| // this is how to get access to custom parameters defined in the config file at qc.tasks.<task_name>.taskParameters | ||
| if (auto param = mCustomParameters.find("myOwnKey"); param != mCustomParameters.end()) { |
There was a problem hiding this comment.
please remove example code
There was a problem hiding this comment.
The problem is I do not know what is artefact and what is really needed. :-) I am removing lines 43 and 44.
| getObjectsManager()->startPublishing(mHistoInputs); | ||
| getObjectsManager()->startPublishing(mHistoClasses); | ||
| try { | ||
| getObjectsManager()->addMetadata(mHistoBC->GetName(), "custom", "34"); |
There was a problem hiding this comment.
I do not know what those lines do, I thought they might be important. Is it ok to delete whole try { } catch { } section (lines 54-65)?
| @@ -0,0 +1,18 @@ | |||
| { | |||
There was a problem hiding this comment.
I have no idea, I do not even know how it got there. I have removed it.
|
do not forget to push your changes :) |
Very first version of the QC CTP - the code can read raw data file and publish BC, trigger Inputs and trigger classes distributions.