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

WIP OCTRL-917 add public "bookkept" flag to enable bookkeeping of an env #662

Closed
wants to merge 1 commit into from

Conversation

knopers8
Copy link
Collaborator

No description provided.

@knopers8 knopers8 changed the title OCTRL-917 add public "bookkept" flag to enable bookkeeping of an env WIP OCTRL-917 add public "bookkept" flag to enable bookkeeping of an env Sep 18, 2024
@@ -1,5 +1,6 @@
name: !public readout-dataflow
description: !public "Main workflow template for ALICE data taking"
bookkept: !public "true"
Copy link
Member

Choose a reason for hiding this comment

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

@knopers8 does this new var need to be marked as public ? IIRC, that's to control if the var is displayed in the GUI or not as a widget.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i assumed it could be, since I consider it a high-level property of a workflow template and it could be displayed in one way or another whether the environments started with it are bookkept.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure it should be a widget at all, and in any case, this is not the syntax that would make it a widget. It's not even clear that it's a static property of the workflow. My understanding was that it's not up to the user to choose whether an environment will be bookkept or not, but that it depends on some combination of traits of the workflow itself, and of its runtime configuration. In any case, I don't think this is the syntax we want.

@vascobarroso under which conditions should an environment be bookkept / not be bookkept?

Copy link
Member

Choose a reason for hiding this comment

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

We should store in bookkeeping readout-dataflow envs.

We should NOT store in bookkeeping:

  • CRU related actions
  • qc-server workflows

Copy link
Member

Choose a reason for hiding this comment

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

Are all instances of readout-dataflow to be bookkept, no matter the run type, config etc?

Copy link
Member

Choose a reason for hiding this comment

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

I think so, that's what we do now via gRPC correct ? Or do you see a case where we don't do the gRPC call ?

Copy link
Member

Choose a reason for hiding this comment

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

@vascobarroso whether the gRPC calls are triggered currently depends on bookkeeping_enabled which is fully runtime.

Copy link
Member

Choose a reason for hiding this comment

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

@teo yes but that's just an on/off switch. We can do the same thing for the new flag, but apart from that we always want to store.

@teo
Copy link
Member

teo commented Oct 1, 2024

Closing in favour of #664

@teo teo closed this Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants