Repository navigation
feat: subdirectory for numtracker - #1748
ZohebShaikh wants to merge 7 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1748 +/- ##
==========================================
- Coverage 96.37% 96.34% -0.04%
==========================================
Files 50 50
Lines 3831 3853 +22
==========================================
+ Hits 3692 3712 +20
- Misses 139 141 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
tpoliaw
left a comment
tpoliaw
left a comment
There was a problem hiding this comment.
Is the change to make this usable from the CLI/client going to be its own PR or should that be included here as well?
| return { | ||
| "query": dedent(""" | ||
| mutation{ | ||
| mutation( |
There was a problem hiding this comment.
The constant would also help this stay in sync
…configurable # Conflicts: # src/blueapi/core/context.py # tests/unit_tests/service/test_interface.py
Co-authored-by: Peter Holloway <peter.holloway@diamond.ac.uk>
tpoliaw
left a comment
tpoliaw
left a comment
There was a problem hiding this comment.
At the moment does anything actually use the subdirectory - can the nexus writer handle it?
Another issue, possibly for another PR, is how to handle invalid subdirectories. numtracker blocks anything that isn't a 'normal' relative path. eg ../foo and /foo will be blocked to prevent users writing outside their visit directories but this will only be checked at the start of a run, potentially a long time into a plan running. Is this an issue?
This also gives us a new error case from numtracker which could potentially be handled better although, again, it shouldn't block this PR.
$ blueapi controller run -i cm12345-1 count -a '[["det"]]' --subdirectory '../foo'
Plan failed: RuntimeError: Numtracker error: [{'message': 'Failed to parse "Subdirectory": Segment 0 of path is not valid for a subdirectory', 'locations': [{'line': 10, 'column': 14}], 'path': ['scan']}]
The information is there so it's ok for now but a custom error message probably wouldn't hurt.
| required=True, | ||
| ) | ||
| @click.option( | ||
| "-sub", |
There was a problem hiding this comment.
Single - should only be used for single letter flags. Either -s or --sub
| }} | ||
| }} | ||
| """) | ||
| "query": dedent(CREATE_SCAN_MUTATION), |
There was a problem hiding this comment.
nit: dedent isn't needed, graphql ignores whitespace
closes #1607, #1199