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

Adding OpMon publisher for drunc #339

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

Conversation

wanyunSu
Copy link
Contributor

No description provided.

@wanyunSu wanyunSu requested a review from plasorak January 21, 2025 13:23
@plasorak plasorak marked this pull request as ready for review January 23, 2025 11:02
Copy link
Collaborator

@plasorak plasorak left a comment

Choose a reason for hiding this comment

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

Looks good. Some changes needed with the schema and in the actions.

_context.publisher.publish(
session="test_runnumber_session",
application="test_runnumber_app",
message = TestInfo(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you define a protobuf schema in druncschema rather than using TestInfo?

_context.publisher.publish(
session="test_runnumber_session",
application="test_runnumber_app",
message = TestInfo(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same

@@ -244,6 +247,7 @@ def can_execute_transition(self, source_state, transition) -> bool:


def prepare_transition(self, transition, transition_data, transition_args, ctx=None):
ctx.publisher = self.publisher
Copy link
Collaborator

Choose a reason for hiding this comment

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

https://github.com/DUNE-DAQ/drunc/pull/339/files#diff-1c8bca3bf6f1ba6689f1ffe354b3be800f9bb7e9143ebc3bdbd45bf558f4897dR620 There is no need to do that here, you can directly do ctx.opmon_publisher.publish in the actions, since ctx = controller


self.configuration = conf
self.publisher = publisher
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need this one?

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.

2 participants