add parameters to eve workflow by jmyrcha · Pull Request #6767 · AliceO2Group/AliceO2

jmyrcha · 2021-07-28T14:35:35Z

No description provided.

davidrohr

Hi @jmyrcha , this looks mostly OK. I have a couple of points inline where there is copy&pasted code from the GPU folder, which is not needed, and I think it should be removed.

But more importantly, I think 2 important points for the commissioning are missing:

We need a command line option to optionally limit the maximum number of tracks that can be written in a JSON file.
We need a command line option to make skip the JSON export if the hostname of the node is not equal to a string provided via command line.

added parameters for maximum number of tracks and host filtering

jmyrcha · 2021-07-29T18:00:52Z

I have implemented all points
(current pull request fails, but it looks like errors on the test environment - logs are empty). If there are any action on my side please let me know.

davidrohr

Hi @jmyrcha, thx looks better, checking the code again, I saw one bogus comment:

// Full configuration structure with all available settings of GPU...

And I am listed as author of several of the files you added, could you clean that up as well?

Finally, could you comment when we can have the 2 missing settings, to limit the number of tracks, and to check the hostname?

davidrohr · 2021-07-29T18:03:05Z

@TimoWilken : Could you check why the CI is failing without a log?

jmyrcha · 2021-07-29T18:07:25Z

Missing settings are already there:
eve-hostname - if set then workflows on computers which hostname do not match will do nothing
number-of_tracks - saves no more than given tracks
I will fix comments and author notes

davidrohr · 2021-07-29T18:09:49Z

Missing settings are already there:
eve-hostname - if set then workflows on computers which hostname do not match will do nothing
number-of_tracks - saves no more than given tracks

ok, sorry, I didn't recheck the code since you didn't comment on it. Looks good. Thx

davidrohr

looks good now

TimoWilken · 2021-07-29T23:41:29Z

Sorry for the build failures this evening. I've reset the checks to pending as there weren't any previous statuses to fall back on.

davidrohr · 2021-07-30T07:10:50Z

@jmyrcha : Could you please fix this CI warning:

/sw/SOURCES/O2/6767-slc8_x86-64/0/EventVisualisation/Workflow/src/O2DPLDisplay.cxx:38:10: error: inclusion of deprecated C++ header 'limits.h'; consider using 'climits' instead [modernize-deprecated-headers]

jmyrcha · 2021-07-30T08:42:57Z

is it possible to run fullCl check locally?
If so - how?

davidrohr · 2021-07-30T08:45:41Z

is it possible to run fullCl check locally?
If so - how?

Pull the docker container alisw/slc8-gpu-builder
In the container, export ALIBUILD_O2_TESTS=1
Then use aliBuild to compile O2FullCI inside the container.

jmyrcha · 2021-07-30T11:19:37Z

I have pull container, but then:
docker run -d alisw/slc8-gpu-builder
07175ddb9680d5e85d176aa7eab3ef4054be4709c0f0cf151488c36ec37ee98d
docker ps -a
CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES
07175ddb9680 alisw/slc8-gpu-builder "/bin/sh" 19 seconds ago Exited (0) 18 seconds ago tender_hamilton
bf6d3bce64a1 mcr.microsoft.com/mssql/server:2019-latest "/opt/mssql/bin/perm…" 55 minutes ago Exited (0) 44 minutes ago sqlserver
docker exec -it tender_hamilton "bash"
Error response from daemon: Container 07175ddb9680d5e85d176aa7eab3ef4054be4709c0f0cf151488c36ec37ee98d is not running

how should I run the image? is there any user I should use/configure (passwords?) I guess I should clone my repo inside and build? aliBuild O2fullCl?

jmyrcha · 2021-07-30T11:20:46Z

sorry for bold in comment - it was probably after copy from console

davidrohr · 2021-07-30T11:33:22Z

Hi @jmyrcha : I am no docker expert, but I am usually doing docker run -ti --shm-size 32G alisw/slc8-gpu-builder.

* add parameters to eve workflow * removed dependency from gpu added parameters for maximum number of tracks and host filtering * reformating * reformating * fixed fullCl warning

add parameters to eve workflow

3e0c795

davidrohr requested changes Jul 29, 2021

View reviewed changes

jmyrcha added 2 commits July 29, 2021 19:45

removed dependency from gpu

6aec890

added parameters for maximum number of tracks and host filtering

reformating

8636c19

jmyrcha requested a review from davidrohr July 29, 2021 17:58

davidrohr requested changes Jul 29, 2021

View reviewed changes

reformating

4375320

jmyrcha requested a review from davidrohr July 29, 2021 18:28

davidrohr previously approved these changes Jul 29, 2021

View reviewed changes

fixed fullCl warning

384292d

jmyrcha dismissed davidrohr’s stale review via 384292d July 30, 2021 08:36

davidrohr merged commit 4ed4115 into AliceO2Group:dev Jul 30, 2021

jmyrcha deleted the eve-workflow-parameters branch September 30, 2021 07:14

Uh oh!

Conversation

jmyrcha commented Jul 28, 2021

Uh oh!

davidrohr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jmyrcha commented Jul 29, 2021

Uh oh!

davidrohr left a comment

Choose a reason for hiding this comment

Uh oh!

davidrohr commented Jul 29, 2021

Uh oh!

jmyrcha commented Jul 29, 2021

Uh oh!

davidrohr commented Jul 29, 2021

Uh oh!

davidrohr left a comment

Choose a reason for hiding this comment

Uh oh!

TimoWilken commented Jul 29, 2021

Uh oh!

davidrohr commented Jul 30, 2021

Uh oh!

jmyrcha commented Jul 30, 2021

Uh oh!

davidrohr commented Jul 30, 2021

Uh oh!

jmyrcha commented Jul 30, 2021

Uh oh!

jmyrcha commented Jul 30, 2021

Uh oh!

davidrohr commented Jul 30, 2021

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants