Feature/v4 default scans - #1010
Conversation
0707b6d to
56772b6
Compare
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
9799fd8 to
a1a5794
Compare
There was a problem hiding this comment.
Pull request overview
This PR migrates the default/internal scan surface from legacy (v3) naming to v4 scans by removing the _v4_ prefixes, updating client/server handling to prefer v4 implementations, and introducing scan-info flags (is_internal, is_scan) to support downstream UI filtering.
Changes:
- Rename v4 scan
scan_names to their canonical (non-_v4_) names and update unit/e2e tests accordingly. - Update scan discovery to prefer v4 scans when names collide, and publish
is_internal/is_scaninavailable_scans. - Update client scan exposure and device RPC request shape to align with the v4-style parameter format.
Reviewed changes
Copilot reviewed 67 out of 67 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pytest_bec_e2e/pytest_bec_e2e/plugin.py | Docstring formatting cleanup. |
| bec_server/tests/tests_scan_server/test_scan_server_scan_manager.py | Adds coverage for scan discovery preferring v4 names on collisions. |
| bec_server/tests/tests_scan_server/test_scan_server_queue.py | Updates queue-related tests for renamed scans and v4 queue item model changes. |
| bec_server/tests/tests_scan_server/test_scan_guard.py | Updates device_rpc request shape expectations and helpers. |
| bec_server/tests/tests_scan_server/test_scan_assembler.py | Updates assembler tests to use non-_v4_ scan names. |
| bec_server/tests/tests_scan_server/test_scan_argument_modifier.py | Updates generated doc example to non-_v4_ names. |
| bec_server/tests/tests_scan_server/scans_v4/test_updated_move_scan.py | Updates v4 scan tests to use renamed umv. |
| bec_server/tests/tests_scan_server/scans_v4/test_time_scan.py | Updates v4 scan tests to use renamed time_scan. |
| bec_server/tests/tests_scan_server/scans_v4/test_scan_components.py | Updates v4 scan tests to use renamed mv. |
| bec_server/tests/tests_scan_server/scans_v4/test_round_scan.py | Updates v4 scan tests to use renamed round_scan. |
| bec_server/tests/tests_scan_server/scans_v4/test_round_roi_scan.py | Updates v4 scan tests to use renamed round_roi_scan. |
| bec_server/tests/tests_scan_server/scans_v4/test_relative_scan_return_to_start.py | Expands/updates relative scan cases for renamed scan types. |
| bec_server/tests/tests_scan_server/scans_v4/test_multi_region_line_scan.py | Updates v4 scan tests to use renamed multi_region_line_scan. |
| bec_server/tests/tests_scan_server/scans_v4/test_multi_region_grid_scan.py | Updates v4 scan tests to use renamed multi_region_grid_scan. |
| bec_server/tests/tests_scan_server/scans_v4/test_move_scan.py | Updates v4 scan tests to use renamed mv. |
| bec_server/tests/tests_scan_server/scans_v4/test_log_scan.py | Updates v4 scan tests to use renamed log_scan. |
| bec_server/tests/tests_scan_server/scans_v4/test_list_scan.py | Updates v4 scan tests to use renamed list_scan. |
| bec_server/tests/tests_scan_server/scans_v4/test_line_sweep_scan.py | Updates v4 scan tests to use renamed line_sweep_scan. |
| bec_server/tests/tests_scan_server/scans_v4/test_line_scan.py | Updates v4 scan tests to use renamed line_scan. |
| bec_server/tests/tests_scan_server/scans_v4/test_hexagonal_scan.py | Updates v4 scan tests to use renamed hexagonal_scan. |
| bec_server/tests/tests_scan_server/scans_v4/test_grid_scan.py | Updates v4 scan tests to use renamed grid_scan. |
| bec_server/tests/tests_scan_server/scans_v4/test_fermat_scan.py | Updates v4 scan tests to use renamed fermat_scan. |
| bec_server/tests/tests_scan_server/scans_v4/test_device_rpc_scan.py | Updates v4 scan tests to use renamed device_rpc. |
| bec_server/tests/tests_scan_server/scans_v4/test_cont_line_scan.py | Updates v4 scan tests to use renamed cont_line_scan. |
| bec_server/tests/tests_scan_server/scans_v4/test_acquire.py | Updates v4 scan tests to use renamed acquire. |
| bec_server/bec_server/scihub/atlas/atlas_metadata_handler.py | Minor formatting/parentheses update in message creation. |
| bec_server/bec_server/scan_server/scans/updated_move.py | Renames v4 umv scan_name (drops _v4_ prefix). |
| bec_server/bec_server/scan_server/scans/time_scan.py | Renames v4 time_scan scan_name (drops _v4_ prefix). |
| bec_server/bec_server/scan_server/scans/scan_base.py | Renames v4 base scan_name (drops _v4_ prefix). |
| bec_server/bec_server/scan_server/scans/scan_argument_modifier.py | Minor formatting/parentheses updates. |
| bec_server/bec_server/scan_server/scans/round_scan.py | Renames v4 round_scan scan_name (drops _v4_ prefix). |
| bec_server/bec_server/scan_server/scans/round_roi_scan.py | Renames v4 round_roi_scan scan_name (drops _v4_ prefix). |
| bec_server/bec_server/scan_server/scans/multi_region_line_scan.py | Renames v4 multi_region_line_scan scan_name (drops _v4_ prefix). |
| bec_server/bec_server/scan_server/scans/multi_region_grid_scan.py | Renames v4 multi_region_grid_scan scan_name (drops _v4_ prefix). |
| bec_server/bec_server/scan_server/scans/move.py | Renames v4 mv scan_name (drops _v4_ prefix). |
| bec_server/bec_server/scan_server/scans/log_scan.py | Renames v4 log_scan scan_name (drops _v4_ prefix). |
| bec_server/bec_server/scan_server/scans/list_scan.py | Renames v4 list_scan scan_name (drops _v4_ prefix). |
| bec_server/bec_server/scan_server/scans/line_sweep_scan.py | Renames v4 line_sweep_scan scan_name (drops _v4_ prefix). |
| bec_server/bec_server/scan_server/scans/line_scan.py | Renames v4 line_scan scan_name (drops _v4_ prefix). |
| bec_server/bec_server/scan_server/scans/hexagonal_scan.py | Renames v4 hexagonal_scan scan_name (drops _v4_ prefix). |
| bec_server/bec_server/scan_server/scans/grid_scan.py | Renames v4 grid_scan scan_name (drops _v4_ prefix). |
| bec_server/bec_server/scan_server/scans/fermat_scan.py | Renames v4 fermat_scan scan_name (drops _v4_ prefix). |
| bec_server/bec_server/scan_server/scans/device_rpc.py | Renames v4 device_rpc scan_name (drops _v4_ prefix). |
| bec_server/bec_server/scan_server/scans/cont_line_scan.py | Renames v4 cont_line_scan scan_name (drops _v4_ prefix). |
| bec_server/bec_server/scan_server/scans/acquire.py | Renames v4 acquire scan_name (drops _v4_ prefix). |
| bec_server/bec_server/scan_server/scan_manager.py | Prefers v4 scans on name collisions; publishes is_scan/is_internal. |
| bec_server/bec_server/scan_server/scan_guard.py | Removes legacy/v4 branching for device_rpc; assumes v4-style parameter shape. |
| bec_server/bec_server/procedures/manager.py | Minor formatting/docstring wrapping updates. |
| bec_server/bec_server/procedures/constants.py | Minor docstring wrapping updates. |
| bec_lib/tests/test_scan_object.py | Removes tests tied to scan_group/scan_def_id propagation. |
| bec_lib/tests/test_scan_items.py | Removes tests tied to scan_def_id tracking. |
| bec_lib/tests/test_scan_context.py | Removes scan_group/interactive scan tests and unused imports. |
| bec_lib/tests/test_devices.py | Updates device_rpc expectations; introduces a test name typo. |
| bec_lib/tests/test_config_helper.py | Minor formatting/parentheses update. |
| bec_lib/bec_lib/tests/utils.py | Removes scan_group-related mock surface. |
| bec_lib/bec_lib/signature_serializer.py | Minor formatting/parentheses updates. |
| bec_lib/bec_lib/scans.py | Filters out is_internal scans from client surface; removes scan_group/interactive scan support and related metadata propagation. |
| bec_lib/bec_lib/scan_report.py | Removes special-casing for _v4_mv now that mv is canonical. |
| bec_lib/bec_lib/scan_items.py | Removes scan_def_id tracking in scan storage updates. |
| bec_lib/bec_lib/scan_data_container.py | Minor formatting/parentheses update. |
| bec_lib/bec_lib/redis_connector/streams.py | Minor docstring wrapping update. |
| bec_lib/bec_lib/device.py | Switches RPC scan_type to canonical device_rpc and v4-style parameter packing; removes scan_def_id propagation. |
| bec_ipython_client/tests/end-2-end/test_scans_v4_lib_e2e.py | Uses canonical scan names and updates test function names. |
| bec_ipython_client/tests/end-2-end/test_scans_lib_e2e.py | Removes dual v3/v4 parametrization; uses canonical scan names only. |
| bec_ipython_client/tests/end-2-end/test_scans_e2e.py | Removes dual v3/v4 parametrization; uses canonical scan names only; updates expected behavior. |
| bec_ipython_client/demo.py | Removes commented examples related to scan_group/interactive scans. |
| bec_ipython_client/bec_ipython_client/callbacks/ipython_live_updates.py | Simplifies report-instruction processing; removes scan_def/interactive branching and _v4_mv handling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 75 out of 75 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (4)
bec_server/bec_server/scan_server/scan_manager.py:197
is_internalcurrently ignoresINTERNAL_SCAN_CLASSESfor scans that inheritScanBasebecauseScanBasedefinesis_internal=False, sohasattr(scan_cls, "is_internal")is always true. This meansDeviceRpc(and any other internal allowlisted scans) will be published as non-internal and therefore exposed to clients despite the intent to hide it.
"is_scan": scan_cls.is_scan if hasattr(scan_cls, "is_scan") else False,
"is_internal": (
scan_cls.is_internal
if hasattr(scan_cls, "is_internal")
else scan_cls.__name__ in INTERNAL_SCAN_CLASSES
),
bec_server/bec_server/scan_server/scans/device_rpc.py:42
DeviceRpcis intended to be an internal control scan (hidden from the user namespace), but it does not setis_internal=True. Relying on class-name allowlists is brittle; marking it explicitly makes the intent unambiguous and ensures clients can reliably filter it out.
bec_server/bec_server/bec_server_utils/subprocess_launch.py:86- After a timeout,
_stop_subprocess()kills the process and then callswait(timeout=timeout_s)again, but a secondTimeoutExpiredwill bubble up and can break teardown/cleanup (leaving child processes running). It’s safer to swallow a second timeout (or log) after a best-effort kill.
except subprocess.TimeoutExpired:
process.kill()
process.wait(timeout=timeout_s)
bec_server/bec_server/bec_server_utils/subprocess_launch.py:77
- Similarly,
_stop_psutil_process()will raise if the secondwait(timeout=timeout_s)times out afterkill(), which can makesubprocess_stop()fail during cleanup. Consider handling a secondpsutil.TimeoutExpiredafter kill as a best-effort stop.
except psutil.TimeoutExpired:
process.kill()
try:
process.wait(timeout=timeout_s)
except psutil.NoSuchProcess:
return
dd9c0c3 to
ec5baf5
Compare
wyzula-jan
left a comment
There was a problem hiding this comment.
minor comments, for logging shadowed scans I have fix which I can push
ec5baf5 to
62806bb
Compare
Description
This PR replaces the internal legacy scans with v4 scans and renames them accordingly. The feature parity has already been established with the previous PR (#994).
How to test
Potential side effects
We need to adapt the scan control widget in BW to filter the v4 scans to only show relevant ones. Two new scan info flags have been introduced in this PR to accomplish that: "is_internal" and "is_scan".
Additional Comments
While it does not remove the ability to run legacy scans, it doesn't expose them anymore. Once the OMNY scans are tested on the new system, we can remove them completely.
Definition of Done