-
Notifications
You must be signed in to change notification settings - Fork 5
Controller include/exclude message types (drunc) #669
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
Conversation
f2fc19c to
2f28819
Compare
0660f68 to
7486ab3
Compare
|
I have run the integration tests twice on this codebase. The first time failed with This addressed is from a completely unrelated issue, which I suspect is from multiple testers on the same physical host with a static connectivity service address. Giving a few mins break and rerunning the tests, I get the following on I will announce the merge shortly on # daq-release-preparation. |
|
This code is also much cleaner now, and highlights just how many methods still require implementation, e.g. a formal |
PawelPlesniak
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you!
|
It is also nice to see when using |
|
Hi James, there is immediately an issue when running |
|
Looks much better There is still a stray log, I removed it in the commit |
Description
Fixes #667.
Few things going on here:
include/excludeRPCs using dedicated message typesRecomputeStatusResponse->StatusResponseChildNodeis now a pure abstract class -- all subclasses MUST reimplement all methods explicitly. This was done to help reduce cognitive load -- better a bit extra verbosity to make the code both safer and easier to read.Sister
druncschemaPR DUNE-DAQ/druncschema#75Unfortunately some extra noise in this PR from things moving around. As usual, happy to chat and walk through.
Type of change
Key checklist
python -m pytest)pre-commit run --all-files)Further checks
(Indicate issue here: # (issue))