From 3a57c91b0da75ead8847dc7e3b09f8305384e045 Mon Sep 17 00:00:00 2001 From: WilliamHPNielsen Date: Mon, 29 Apr 2019 15:16:13 +0200 Subject: [PATCH 1/6] Refactor a check out into a method --- qcodes/dataset/descriptions.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/qcodes/dataset/descriptions.py b/qcodes/dataset/descriptions.py index 09ebf2d5765..a7a268a7282 100644 --- a/qcodes/dataset/descriptions.py +++ b/qcodes/dataset/descriptions.py @@ -59,7 +59,7 @@ def deserialize(cls, ser: Dict[str, Any]) -> 'RunDescriber': idp: Union[InterDependencies, InterDependencies_] - if 'paramspecs' in ser['interdependencies'].keys(): + if cls._is_description_old_style(ser['interdependencies']): idp = InterDependencies.deserialize(ser['interdependencies']) else: idp = InterDependencies_.deserialize(ser['interdependencies']) @@ -67,6 +67,20 @@ def deserialize(cls, ser: Dict[str, Any]) -> 'RunDescriber': return rundesc + @staticmethod + def _is_description_old_style(serialized_object: Dict[str, Any]) -> bool: + """ + Returns True if an old style description is encountered + """ + + # NOTE: we should probably think carefully about versioning; keeping + # the runs description in sync with the API (this file) + + if 'paramspecs' in serialized_object.keys(): + return True + else: + return False + @staticmethod def _ruamel_importer(): try: From 0dc2d8af18a40ab77d82e8af4e1204e8072634b5 Mon Sep 17 00:00:00 2001 From: WilliamHPNielsen Date: Mon, 29 Apr 2019 15:39:50 +0200 Subject: [PATCH 2/6] Add test for fix function --- qcodes/tests/dataset/test_fix_functions.py | 32 ++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 qcodes/tests/dataset/test_fix_functions.py diff --git a/qcodes/tests/dataset/test_fix_functions.py b/qcodes/tests/dataset/test_fix_functions.py new file mode 100644 index 00000000000..99cec611598 --- /dev/null +++ b/qcodes/tests/dataset/test_fix_functions.py @@ -0,0 +1,32 @@ +import os + +import pytest + +from qcodes.dataset.database_fix_functions import fix_version_4a_run_description_bug +import qcodes.tests.dataset +from qcodes.tests.dataset.temporary_databases import temporarily_copied_DB + +fixturepath = os.sep.join(qcodes.tests.dataset.__file__.split(os.sep)[:-1]) +fixturepath = os.path.join(fixturepath, 'fixtures') + + +def test_version_4a_bugfix(): + v1fixpath = os.path.join(fixturepath, 'db_files', 'version4a') + + dbname_old = os.path.join(v1fixpath, 'some_runs.db') + + if not os.path.exists(dbname_old): + pytest.skip("No db-file fixtures found. You can generate test db-files" + " using the scripts in the legacy_DB_generation folder") + + with temporarily_copied_DB(dbname_old, debug=False, version=4) as conn: + + dd = fix_version_4a_run_description_bug(conn) + + assert dd['runs_inspected'] == 10 + assert dd['runs_fixed'] == 10 + + dd = fix_version_4a_run_description_bug(conn) + + assert dd['runs_inspected'] == 10 + assert dd['runs_fixed'] == 0 From 93a58915d87d1b99ea9e1221685be13814592a9b Mon Sep 17 00:00:00 2001 From: WilliamHPNielsen Date: Mon, 29 Apr 2019 15:40:02 +0200 Subject: [PATCH 3/6] Add fix function --- qcodes/dataset/database_fix_functions.py | 64 ++++++++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 qcodes/dataset/database_fix_functions.py diff --git a/qcodes/dataset/database_fix_functions.py b/qcodes/dataset/database_fix_functions.py new file mode 100644 index 00000000000..254a5883dd7 --- /dev/null +++ b/qcodes/dataset/database_fix_functions.py @@ -0,0 +1,64 @@ +""" +Sometimes it happens that databases are put into inconsistent/corrupt states. +This module contains functions to remedy known issues. +""" +import json +from typing import Dict + +from tqdm import tqdm + +from qcodes.dataset.sqlite_base import (ConnectionPlus, get_user_version, + atomic) +from qcodes.dataset.descriptions import RunDescriber +from qcodes.dataset.sqlite_base import (get_run_description, + update_run_description, + one, atomic_transaction) + + +def fix_version_4a_run_description_bug(conn: ConnectionPlus) -> Dict[str, int]: + """ + Fix function to fix a bug where the RunDescriber accidentally wrote itself + to string using the (new) InterDependencies_ object instead of the (old) + InterDependencies object. After the first run, this function should be + idempotent. + + + Args: + conn: the connection to the database + + Returns: + A dict with the fix results ('runs_inspected', 'runs_fixed') + """ + + if not get_user_version(conn) == 4: + raise RuntimeError('Database of wrong version. Will not apply fix.') + + no_of_runs_query = "SELECT max(run_id) FROM runs" + no_of_runs = one(atomic_transaction(conn, no_of_runs_query), 'max(run_id)') + no_of_runs = no_of_runs or 0 + + with atomic(conn) as conn: + + pbar = tqdm(range(1, no_of_runs+1)) + pbar.set_description("Fixing database") + + # collect some metrics + runs_inspected = 0 + runs_fixed = 0 + + for run_id in pbar: + + desc_str = get_run_description(conn, run_id) + desc_ser = json.loads(desc_str) + idps_ser = desc_ser['interdependencies'] + + if RunDescriber._is_description_old_style(idps_ser): + pass + else: + new_desc = RunDescriber.from_json(desc_str) + update_run_description(conn, run_id, new_desc.to_json()) + runs_fixed += 1 + + runs_inspected += 1 + + return {'runs_inspected': runs_inspected, 'runs_fixed': runs_fixed} From 06da63af5a252c98d43bbf62e5b0a1d1a3a7f868 Mon Sep 17 00:00:00 2001 From: WilliamHPNielsen Date: Tue, 30 Apr 2019 10:25:28 +0200 Subject: [PATCH 4/6] Move fix functions and their tests --- qcodes/dataset/database_fix_functions.py | 52 ++++++++++++++++++--- qcodes/dataset/sqlite_base.py | 34 -------------- qcodes/tests/dataset/test_database_fixes.py | 45 ------------------ qcodes/tests/dataset/test_fix_functions.py | 39 +++++++++++++++- 4 files changed, 84 insertions(+), 86 deletions(-) delete mode 100644 qcodes/tests/dataset/test_database_fixes.py diff --git a/qcodes/dataset/database_fix_functions.py b/qcodes/dataset/database_fix_functions.py index 254a5883dd7..4359802a2cb 100644 --- a/qcodes/dataset/database_fix_functions.py +++ b/qcodes/dataset/database_fix_functions.py @@ -3,16 +3,22 @@ This module contains functions to remedy known issues. """ import json -from typing import Dict +import logging +from typing import Dict, Sequence from tqdm import tqdm -from qcodes.dataset.sqlite_base import (ConnectionPlus, get_user_version, - atomic) from qcodes.dataset.descriptions import RunDescriber -from qcodes.dataset.sqlite_base import (get_run_description, - update_run_description, - one, atomic_transaction) +from qcodes.dataset.dependencies import InterDependencies +from qcodes.dataset.sqlite_base import (ConnectionPlus, atomic, + atomic_transaction, + get_parameters, + get_run_description, get_user_version, + one, + select_one_where, + update_run_description) + +log = logging.getLogger(__name__) def fix_version_4a_run_description_bug(conn: ConnectionPlus) -> Dict[str, int]: @@ -62,3 +68,37 @@ def fix_version_4a_run_description_bug(conn: ConnectionPlus) -> Dict[str, int]: runs_inspected += 1 return {'runs_inspected': runs_inspected, 'runs_fixed': runs_fixed} + + +def fix_wrong_run_descriptions(conn: ConnectionPlus, + run_ids: Sequence[int]) -> None: + """ + NB: This is a FIX function. Do not use it unless your database has been + diagnosed with the problem that this function fixes. + + Overwrite faulty run_descriptions by using information from the layouts and + dependencies tables. If a correct description is found for a run, that + run is left untouched. + + Args: + conn: The connection to the database + run_ids: The runs to (potentially) fix + """ + + log.info('[*] Fixing run descriptions...') + for run_id in run_ids: + trusted_paramspecs = get_parameters(conn, run_id) + trusted_desc = RunDescriber( + interdeps=InterDependencies(*trusted_paramspecs)) + + actual_desc_str = select_one_where(conn, "runs", + "run_description", + "run_id", run_id) + + if actual_desc_str == trusted_desc.to_json(): + log.info(f'[+] Run id: {run_id} had an OK description') + else: + log.info(f'[-] Run id: {run_id} had a broken description. ' + f'Description found: {actual_desc_str}') + update_run_description(conn, run_id, trusted_desc.to_json()) + log.info(f' Run id: {run_id} has been updated.') diff --git a/qcodes/dataset/sqlite_base.py b/qcodes/dataset/sqlite_base.py index 3cc55101146..bd30eace430 100644 --- a/qcodes/dataset/sqlite_base.py +++ b/qcodes/dataset/sqlite_base.py @@ -2706,37 +2706,3 @@ def remove_trigger(conn: ConnectionPlus, trigger_id: str) -> None: name: id of the trigger """ transaction(conn, f"DROP TRIGGER IF EXISTS {trigger_id};") - - -def _fix_wrong_run_descriptions(conn: ConnectionPlus, - run_ids: Sequence[int]) -> None: - """ - NB: This is a FIX function. Do not use it unless your database has been - diagnosed with the problem that this function fixes. - - Overwrite faulty run_descriptions by using information from the layouts and - dependencies tables. If a correct description is found for a run, that - run is left untouched. - - Args: - conn: The connection to the database - run_ids: The runs to (potentially) fix - """ - - log.info('[*] Fixing run descriptions...') - for run_id in run_ids: - trusted_paramspecs = get_parameters(conn, run_id) - trusted_desc = RunDescriber( - interdeps=InterDependencies(*trusted_paramspecs)) - - actual_desc_str = select_one_where(conn, "runs", - "run_description", - "run_id", run_id) - - if actual_desc_str == trusted_desc.to_json(): - log.info(f'[+] Run id: {run_id} had an OK description') - else: - log.info(f'[-] Run id: {run_id} had a broken description. ' - f'Description found: {actual_desc_str}') - update_run_description(conn, run_id, trusted_desc.to_json()) - log.info(f' Run id: {run_id} has been updated.') diff --git a/qcodes/tests/dataset/test_database_fixes.py b/qcodes/tests/dataset/test_database_fixes.py deleted file mode 100644 index 51a559f79ae..00000000000 --- a/qcodes/tests/dataset/test_database_fixes.py +++ /dev/null @@ -1,45 +0,0 @@ -import os - -import pytest - -import qcodes -from qcodes.dataset.data_set import DataSet -from qcodes.dataset.dependencies import InterDependencies_ -from qcodes.dataset.descriptions import RunDescriber -from qcodes.dataset.sqlite_base import _fix_wrong_run_descriptions, \ - get_user_version -from qcodes.tests.dataset.temporary_databases import temporarily_copied_DB - -fixturepath = os.sep.join(qcodes.tests.dataset.__file__.split(os.sep)[:-1]) -fixturepath = os.path.join(fixturepath, 'fixtures') - - -def test_fix_wrong_run_descriptions(): - v3fixpath = os.path.join(fixturepath, 'db_files', 'version3') - - dbname_old = os.path.join(v3fixpath, 'some_runs_without_run_description.db') - - if not os.path.exists(dbname_old): - pytest.skip( - "No db-file fixtures found. You can generate test db-files" - " using the scripts in the legacy_DB_generation folder") - - with temporarily_copied_DB(dbname_old, debug=False, version=3) as conn: - - assert get_user_version(conn) == 3 - - ds1 = DataSet(conn=conn, run_id=1) - expected_description = ds1.description - - empty_description = RunDescriber(InterDependencies_()) - - _fix_wrong_run_descriptions(conn, [1, 2, 3, 4]) - - ds2 = DataSet(conn=conn, run_id=2) - assert expected_description == ds2.description - - ds3 = DataSet(conn=conn, run_id=3) - assert expected_description == ds3.description - - ds4 = DataSet(conn=conn, run_id=4) - assert empty_description == ds4.description diff --git a/qcodes/tests/dataset/test_fix_functions.py b/qcodes/tests/dataset/test_fix_functions.py index 99cec611598..f732826ddd0 100644 --- a/qcodes/tests/dataset/test_fix_functions.py +++ b/qcodes/tests/dataset/test_fix_functions.py @@ -2,9 +2,15 @@ import pytest -from qcodes.dataset.database_fix_functions import fix_version_4a_run_description_bug +from qcodes.dataset.database_fix_functions import ( + fix_version_4a_run_description_bug, fix_wrong_run_descriptions) import qcodes.tests.dataset from qcodes.tests.dataset.temporary_databases import temporarily_copied_DB +from qcodes.dataset.data_set import DataSet +from qcodes.dataset.dependencies import InterDependencies_ +from qcodes.dataset.descriptions import RunDescriber +from qcodes.dataset.sqlite_base import _fix_wrong_run_descriptions, \ + get_user_version fixturepath = os.sep.join(qcodes.tests.dataset.__file__.split(os.sep)[:-1]) fixturepath = os.path.join(fixturepath, 'fixtures') @@ -30,3 +36,34 @@ def test_version_4a_bugfix(): assert dd['runs_inspected'] == 10 assert dd['runs_fixed'] == 0 + + +def test_fix_wrong_run_descriptions(): + v3fixpath = os.path.join(fixturepath, 'db_files', 'version3') + + dbname_old = os.path.join(v3fixpath, 'some_runs_without_run_description.db') + + if not os.path.exists(dbname_old): + pytest.skip( + "No db-file fixtures found. You can generate test db-files" + " using the scripts in the legacy_DB_generation folder") + + with temporarily_copied_DB(dbname_old, debug=False, version=3) as conn: + + assert get_user_version(conn) == 3 + + ds1 = DataSet(conn=conn, run_id=1) + expected_description = ds1.description + + empty_description = RunDescriber(InterDependencies_()) + + fix_wrong_run_descriptions(conn, [1, 2, 3, 4]) + + ds2 = DataSet(conn=conn, run_id=2) + assert expected_description == ds2.description + + ds3 = DataSet(conn=conn, run_id=3) + assert expected_description == ds3.description + + ds4 = DataSet(conn=conn, run_id=4) + assert empty_description == ds4.description From 00d88bdfeb4cc433e1dceab5cc19a8e1e8757b70 Mon Sep 17 00:00:00 2001 From: WilliamHPNielsen Date: Tue, 30 Apr 2019 10:34:55 +0200 Subject: [PATCH 5/6] Make fix functions raise if db version is wrong --- qcodes/dataset/database_fix_functions.py | 14 ++++++-- qcodes/tests/dataset/test_fix_functions.py | 37 +++++++++++++++++++--- 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/qcodes/dataset/database_fix_functions.py b/qcodes/dataset/database_fix_functions.py index 4359802a2cb..18d3b7df033 100644 --- a/qcodes/dataset/database_fix_functions.py +++ b/qcodes/dataset/database_fix_functions.py @@ -36,8 +36,11 @@ def fix_version_4a_run_description_bug(conn: ConnectionPlus) -> Dict[str, int]: A dict with the fix results ('runs_inspected', 'runs_fixed') """ - if not get_user_version(conn) == 4: - raise RuntimeError('Database of wrong version. Will not apply fix.') + user_version = get_user_version(conn) + + if not user_version == 4: + raise RuntimeError('Database of wrong version. Will not apply fix. ' + 'Expected version 4, found version {user_version}') no_of_runs_query = "SELECT max(run_id) FROM runs" no_of_runs = one(atomic_transaction(conn, no_of_runs_query), 'max(run_id)') @@ -85,6 +88,13 @@ def fix_wrong_run_descriptions(conn: ConnectionPlus, run_ids: The runs to (potentially) fix """ + user_version = get_user_version(conn) + + if not user_version == 3: + raise RuntimeError('Database of wrong version. Will not apply fix. ' + 'Expected version 3, found version {user_version}') + + log.info('[*] Fixing run descriptions...') for run_id in run_ids: trusted_paramspecs = get_parameters(conn, run_id) diff --git a/qcodes/tests/dataset/test_fix_functions.py b/qcodes/tests/dataset/test_fix_functions.py index f732826ddd0..6db0de6114a 100644 --- a/qcodes/tests/dataset/test_fix_functions.py +++ b/qcodes/tests/dataset/test_fix_functions.py @@ -9,17 +9,16 @@ from qcodes.dataset.data_set import DataSet from qcodes.dataset.dependencies import InterDependencies_ from qcodes.dataset.descriptions import RunDescriber -from qcodes.dataset.sqlite_base import _fix_wrong_run_descriptions, \ - get_user_version +from qcodes.dataset.sqlite_base import get_user_version fixturepath = os.sep.join(qcodes.tests.dataset.__file__.split(os.sep)[:-1]) fixturepath = os.path.join(fixturepath, 'fixtures') def test_version_4a_bugfix(): - v1fixpath = os.path.join(fixturepath, 'db_files', 'version4a') + v4fixpath = os.path.join(fixturepath, 'db_files', 'version4a') - dbname_old = os.path.join(v1fixpath, 'some_runs.db') + dbname_old = os.path.join(v4fixpath, 'some_runs.db') if not os.path.exists(dbname_old): pytest.skip("No db-file fixtures found. You can generate test db-files" @@ -38,6 +37,21 @@ def test_version_4a_bugfix(): assert dd['runs_fixed'] == 0 +def test_version_4a_bugfix_raises(): + + v3fixpath = os.path.join(fixturepath, 'db_files', 'version3') + dbname_old = os.path.join(v3fixpath, 'some_runs_without_run_description.db') + + if not os.path.exists(dbname_old): + pytest.skip( + "No db-file fixtures found. You can generate test db-files" + " using the scripts in the legacy_DB_generation folder") + + with temporarily_copied_DB(dbname_old, debug=False, version=3) as conn: + with pytest.raises(RuntimeError): + fix_version_4a_run_description_bug(conn) + + def test_fix_wrong_run_descriptions(): v3fixpath = os.path.join(fixturepath, 'db_files', 'version3') @@ -67,3 +81,18 @@ def test_fix_wrong_run_descriptions(): ds4 = DataSet(conn=conn, run_id=4) assert empty_description == ds4.description + + +def test_fix_wrong_run_descriptions_raises(): + + v4fixpath = os.path.join(fixturepath, 'db_files', 'version4a') + + dbname_old = os.path.join(v4fixpath, 'some_runs.db') + + if not os.path.exists(dbname_old): + pytest.skip("No db-file fixtures found. You can generate test db-files" + " using the scripts in the legacy_DB_generation folder") + + with temporarily_copied_DB(dbname_old, debug=False, version=4) as conn: + with pytest.raises(RuntimeError): + fix_wrong_run_descriptions(conn, [1]) From 4dabde381feca95800bcad7d0559a50179de6290 Mon Sep 17 00:00:00 2001 From: WilliamHPNielsen Date: Tue, 30 Apr 2019 10:37:38 +0200 Subject: [PATCH 6/6] Make CI generate version 4a fixture --- .travis.yml | 1 + azure-pipelines.yml | 1 + 2 files changed, 2 insertions(+) diff --git a/.travis.yml b/.travis.yml index dde2e6299a5..e26692863e1 100644 --- a/.travis.yml +++ b/.travis.yml @@ -49,6 +49,7 @@ script: python generate_version_1.py python generate_version_2.py python generate_version_3.py + python generate_version_4a.py cd $TRAVIS_BUILD_DIR pip uninstall -y qcodes pip install . diff --git a/azure-pipelines.yml b/azure-pipelines.yml index b6663caccbd..426ad4d4e6d 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -41,6 +41,7 @@ jobs: python generate_version_1.py python generate_version_2.py python generate_version_3.py + python generate_version_4a.py displayName: "Generate db fixtures" condition: succeededOrFailed() - script: |