Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ctpdev: removing dependencies on RunManager #2367

Merged
merged 3 commits into from
Jul 10, 2024

Conversation

lietava
Copy link
Contributor

@lietava lietava commented Jul 4, 2024

Removing dependency on RunManager as discussed in
AliceO2Group/AliceO2#13224
I left old lines commented as it may be moved back after O2 update.

@lietava
Copy link
Contributor Author

lietava commented Jul 5, 2024

Hi @knopers8 @Barthelemy , please merge or comment.

@Barthelemy
Copy link
Collaborator

@lietava We might take a couple of days to review, but we will do it, no worries.

Copy link
Collaborator

@Barthelemy Barthelemy left a comment

Choose a reason for hiding this comment

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

Please consider my comments. i understand that you need to merge it but perhaps you can make a second PR to fix that.

@@ -18,7 +18,7 @@
#include "QualityControl/MonitorObject.h"
#include "QualityControl/Quality.h"
#include "QualityControl/QcInfoLogger.h"
#include "DataFormatsCTP/RunManager.h"
// #include "DataFormatsCTP/RunManager.h"
Copy link
Collaborator

Choose a reason for hiding this comment

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

please, remove lines instead of commenting. The history is any ways kept in git.

metadata["runNumber"] = run;
auto ctpconfigdb = mgr.getSpecific<o2::ctp::CTPConfiguration>(o2::ctp::CCDBPathCTPConfig, mTimestamp, metadata);
if (ctpconfigdb == nullptr) {
LOG(info) << "CTP config not in database, timestamp:" << mTimestamp;
Copy link
Collaborator

Choose a reason for hiding this comment

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

in QC we favour using ILOG i.e. the infologger

map<string, string> metadata; // can be empty
metadata["runNumber"] = run;
auto ctpconfigdb = mgr.getSpecific<o2::ctp::CTPConfiguration>(o2::ctp::CCDBPathCTPConfig, mTimestamp, metadata);
if (ctpconfigdb == nullptr) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

the logic with the ok seems circumvoluted...

@Barthelemy Barthelemy merged commit befd114 into AliceO2Group:master Jul 10, 2024
7 checks passed
@lietava
Copy link
Contributor Author

lietava commented Jul 11, 2024

@Barthelemy thanks for your comments and merge. I will try to implement comments on the next occasion.

@lietava lietava deleted the ctpdev branch August 29, 2024 16:30
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.

2 participants