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

Add rundoc URLConfig protocol #1135

Merged
merged 8 commits into from
Jun 19, 2023
Merged

Add rundoc URLConfig protocol #1135

merged 8 commits into from
Jun 19, 2023

Conversation

jmosbacher
Copy link
Contributor

Inspired by this thread.

This PR add a URLConfig protocol that fetches a value from the rundoc.

The protocol accepts the path to the value inside the rundoc and returns only the value if it exists.

e.g.

URL = "rundoc://source?run_id=plugin.run_id"

will return the source from the rundoc for the run_id set on the plugin.

the path can also be set to access a nested data structure using the standard mongo dot notation.

e.g.

URL = "rundoc://daq_config.boards?run_id=plugin.run_id"

will return the array rundoc["daq_config"]["boards"].

@jmosbacher
Copy link
Contributor Author

Still need to add tests, then can mark it ready for review.

@coveralls
Copy link

coveralls commented Feb 7, 2023

Coverage Status

coverage: 93.417% (+0.02%) from 93.396% when pulling fcad1b7 on add_rundoc_protocol into f728169 on master.

@jmosbacher jmosbacher marked this pull request as ready for review February 16, 2023 10:57
Copy link
Contributor

@JoranAngevaare JoranAngevaare 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, although we might want to check with simulations experts @terliuk since this will for sure break MC again since there is no int('mc_run_aweseome')

We can merge it as it is (no defaults are currently changed) but would have to change the simulation context explicitly when we add this feature in the data stream.

straxen/url_config.py Outdated Show resolved Hide resolved
@jmosbacher
Copy link
Contributor Author

@JoranAngevaare but doesnt the plugin run get set with the "cmt_run_id" value? which is an int no?

@JoranAngevaare
Copy link
Contributor

@jmosbacher not entirely sure anymore, this is not a CMT option, so no I don't think so? I think this would cause errors if used directly.

It by the way doesn't allow setting the thresholds yet based on the rundoc because of the way we organized the rundoc

@jmosbacher
Copy link
Contributor Author

Yeah I am now not so sure this is useful for the thresholds specifically

@dachengx dachengx self-requested a review May 12, 2023 12:47
@dachengx
Copy link
Collaborator

I am curious about what the 'thresholds' are. Is this PR now in good shape?

@jmosbacher
Copy link
Contributor Author

@dachengx this was created initially because the pmt thresholds are stored by the DAQ in the run doc but the plugin actually reads them from CMT. I think this was setup because people thought that we may want to use different thresholds for processing than the ones used by DAQ. In the end we just ended up with the same values being in both places and in some cases not matching due to an error in the cmt db.

This PR adds a protocol that allows you to use values from the run doc in you plugin configuration. Im not sure its really needed.

Copy link
Collaborator

@dachengx dachengx left a comment

Choose a reason for hiding this comment

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

Thanks @jmosbacher . Looks good. Let's go with it.

@dachengx dachengx merged commit 0a2cecc into master Jun 19, 2023
@dachengx dachengx deleted the add_rundoc_protocol branch June 19, 2023 21:14
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.

4 participants