Skip to content

Address review feedback on split raw data storage (PR #8219)#8302

Closed
astafan8 with Copilot wants to merge 18 commits into
mainfrom
copilot/fix-code-for-review-comments
Closed

Address review feedback on split raw data storage (PR #8219)#8302
astafan8 with Copilot wants to merge 18 commits into
mainfrom
copilot/fix-code-for-review-comments

Conversation

Copilot AI commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Applies fixes for the review thread on #8219, incorporating @astafan8's follow-up on the Copilot AI review comments.

Library helpers no longer print to stdout

purge_orphaned_datasets and cleanup_datasets are split into logging-only core helpers plus thin public callers that own user-facing output — keeping programmatic use quiet while preserving console feedback.

def _purge_orphaned_datasets(db_path, *, dry_run=True) -> PurgeResult:
    ...  # core logic, log.info(...) only

def purge_orphaned_datasets(db_path, *, dry_run=True) -> PurgeResult:
    result = _purge_orphaned_datasets(db_path, dry_run=dry_run)
    print(f"Found {result.total_datasets_with_raw_data} datasets ...")
    return result

Harden DROP TABLE

remove_dataset_from_db now runs _validate_table_name(result_table_name) before interpolating the name into DROP TABLE, guarding against dropping an unintended table if the stored value is corrupt.

Docs

  • dataset_design.rst: corrected module path to qcodes.dataset._raw_data_storage.
  • introduction.rst: removed the duplicated Database-notebook pointer.
  • Fixed update_raw_data_paths docstring typo ("old paths3" → "old paths").

Out of scope

Two Code Review notes on pre-existing #8219 code were left untouched: the get_shaped_parameter_data_for_one_paramtree import (false positive — defined in queries.py) and a docstring spacing nit, neither part of the linked thread.

Mikhail Astafev and others added 16 commits June 12, 2026 13:38
Add optional configuration to write results-table data into individual
per-dataset SQLite files (<guid>.db) while keeping all metadata in the
main database. This keeps the main DB lightweight as it grows.

Config options (dataset section of qcodesrc.json):
  - raw_data_to_separate_db (bool, default false)
  - raw_data_path (string, default '{db_location}')

Implementation:
  - New module: qcodes.dataset.raw_data_storage (helper functions)
  - DataSet._data_conn property routes reads/writes to correct DB
  - BackgroundWriter supports per-dataset raw data connections
  - Subscriber triggers created on data connection for compatibility
  - Per-dataset DB path persisted in run metadata for auto-reconnect
  - Empty results table schema kept in main DB for compatibility
  - 19 new tests, all existing tests pass unchanged
  - Documentation added to dataset intro, design docs, and Database notebook

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Rename raw_data_storage.py to _raw_data_storage.py (private module)
- Raise FileNotFoundError when per-dataset raw data file is missing
  on load instead of silently falling back to empty main DB table
- Quote column names in create_raw_data_db to handle SQL keyword names
- Close both _raw_data_conn and conn in all tests to prevent leaked
  file handles
- Add test_missing_raw_data_file_raises to verify the error behavior
- Fix import ordering (ruff I001) and raw regex pattern (ruff RUF043)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Implement a helper function that updates the raw_data_db_path metadata
in the main database when individual raw data SQLite files have been
moved to a new location. This mirrors the existing pattern used for
exported netCDF files.

The function:
- Scans all datasets in the main DB that have raw_data_db_path metadata
- For each, checks if the corresponding .db file exists in the new folder
- Updates the stored path in the metadata to point to the new location
- Reports which datasets were updated and which were skipped (file missing)

Exposed as qcodes.dataset.update_raw_data_paths() for user convenience.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Apply ruff format to _raw_data_storage.py (line length adjustments)
- Comment out unused import in Database.ipynb example cell

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add assert statements to narrow path_to_db from str | None to str
before passing to update_raw_data_paths().

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Two new functions for managing per-dataset raw data storage:

- purge_orphaned_datasets(): Finds and removes dataset records from the
  main DB whose raw data files no longer exist on disk. Useful after
  archiving/deleting selected raw data files. Defaults to dry_run=True.

- cleanup_datasets(): Removes datasets (both DB records AND raw data
  files) matching criteria: older_than_days, sample_name (exact match),
  or larger_than_mb. Criteria use AND logic. Defaults to dry_run=True.

Both functions return dataclass results with full details of what was
found/removed. The _remove_dataset_from_db helper correctly deletes
from runs, layouts, dependencies tables and drops the results table.

Documentation added to introduction.rst and Database.ipynb.
9 new tests covering both functions.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…implify

- Move _get_datasets_with_raw_data and _remove_dataset_from_db to
  src/qcodes/dataset/sqlite/queries.py as public functions
- Add print() statements alongside logging for user visibility
- Use 'with closing(conn):' pattern for connection management
- Simplify orphan detection (rely on raw_data_size_bytes is None)
- Simplify cleanup_datasets docstring
- Simplify introduction.rst to reference Database notebook
- Update news fragment with config option name and DB context

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Change per-dataset log.info to log.debug in purge/cleanup/update_paths
- Replace os.remove with Path.unlink() for robustness
- Remove unused os import

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- _extract_single_dataset_into_db: use dataset._data_conn instead of
  dataset.conn so raw data is read from the per-dataset file
- unsubscribe/unsubscribe_all: use _data_conn to remove triggers that
  were created on the raw data connection
- Add tests for extract_runs_into_db and
  export_datasets_and_create_metadata_db with split raw data

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI and others added 2 commits July 17, 2026 17:45
…t/Qcodes into copilot/fix-code-for-review-comments

Co-authored-by: astafan8 <15662810+astafan8@users.noreply.github.com>
… prints, fix docs

Co-authored-by: astafan8 <15662810+astafan8@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix code based on review comments Address review feedback on split raw data storage (PR #8219) Jul 17, 2026
Copilot AI requested a review from astafan8 July 17, 2026 17:51
@astafan8 astafan8 closed this Jul 18, 2026
@astafan8
astafan8 deleted the copilot/fix-code-for-review-comments branch July 18, 2026 08:09
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