From fb3f786c66a2ba49c72e102991ca42f79d0f5328 Mon Sep 17 00:00:00 2001 From: Jeremy Bowman Date: Mon, 2 Jul 2018 16:42:53 -0400 Subject: [PATCH] TE-2583 Avoid redundant bok-choy DB setup steps --- pavelib/database.py | 14 +++++----- pavelib/paver_tests/test_database.py | 6 ++--- pavelib/utils/db_utils.py | 4 ++- scripts/reset-test-db.sh | 39 ++++++++++++++++------------ 4 files changed, 37 insertions(+), 26 deletions(-) diff --git a/pavelib/database.py b/pavelib/database.py index 06fe07fcaf..6ab05b95d7 100644 --- a/pavelib/database.py +++ b/pavelib/database.py @@ -91,14 +91,16 @@ def update_local_bokchoy_db_from_s3(options): fingerprint = fingerprint_bokchoy_db_files(MIGRATION_OUTPUT_FILES, ALL_DB_FILES) fingerprints_match = does_fingerprint_on_disk_match(fingerprint) + # Calculating the fingerprint already reset the DB, so we don't need to + # do it again (hence use_existing_db=True below) if fingerprints_match: - print ("DB cache files match the current migrations.") - reset_test_db(BOKCHOY_DB_FILES, update_cache_files=False) + print("DB cache files match the current migrations.") + reset_test_db(BOKCHOY_DB_FILES, update_cache_files=False, use_existing_db=True) elif is_fingerprint_in_bucket(fingerprint, CACHE_BUCKET_NAME): - print ("Found updated bokchoy db files at S3.") + print("Found updated bokchoy db files at S3.") refresh_bokchoy_db_cache_from_s3(fingerprint, CACHE_BUCKET_NAME, BOKCHOY_DB_FILES) - reset_test_db(BOKCHOY_DB_FILES, update_cache_files=False) + reset_test_db(BOKCHOY_DB_FILES, update_cache_files=False, use_existing_db=True) else: msg = "{} {} {}".format( @@ -106,8 +108,8 @@ def update_local_bokchoy_db_from_s3(options): "Loading the bokchoy db files from disk", "and running migrations." ) - print (msg) - reset_test_db(BOKCHOY_DB_FILES, update_cache_files=True) + print(msg) + reset_test_db(BOKCHOY_DB_FILES, update_cache_files=True, use_existing_db=True) # Check one last time to see if the fingerprint is present in # the s3 bucket. This could occur because the bokchoy job is # sharded and running the same task in parallel diff --git a/pavelib/paver_tests/test_database.py b/pavelib/paver_tests/test_database.py index 0112f2d277..d874c06e06 100644 --- a/pavelib/paver_tests/test_database.py +++ b/pavelib/paver_tests/test_database.py @@ -115,7 +115,7 @@ class TestPaverDatabaseTasks(MockS3Mixin, TestCase): self.assertFalse(_mock_get_file.called) calls = [ call('{}/scripts/reset-test-db.sh --calculate_migrations'.format(Env.REPO_ROOT)), - call('{}/scripts/reset-test-db.sh'.format(Env.REPO_ROOT)) + call('{}/scripts/reset-test-db.sh --use-existing-db'.format(Env.REPO_ROOT)) ] _mock_sh.assert_has_calls(calls) @@ -156,7 +156,7 @@ class TestPaverDatabaseTasks(MockS3Mixin, TestCase): ) calls = [ call('{}/scripts/reset-test-db.sh --calculate_migrations'.format(Env.REPO_ROOT)), - call('{}/scripts/reset-test-db.sh'.format(Env.REPO_ROOT)) + call('{}/scripts/reset-test-db.sh --use-existing-db'.format(Env.REPO_ROOT)) ] _mock_sh.assert_has_calls(calls) @@ -184,7 +184,7 @@ class TestPaverDatabaseTasks(MockS3Mixin, TestCase): database.update_local_bokchoy_db_from_s3() # pylint: disable=no-value-for-parameter calls = [ call('{}/scripts/reset-test-db.sh --calculate_migrations'.format(Env.REPO_ROOT)), - call('{}/scripts/reset-test-db.sh --rebuild_cache'.format(Env.REPO_ROOT)) + call('{}/scripts/reset-test-db.sh --rebuild_cache --use-existing-db'.format(Env.REPO_ROOT)) ] _mock_sh.assert_has_calls(calls) diff --git a/pavelib/utils/db_utils.py b/pavelib/utils/db_utils.py index bf3f3e82f3..182e12e1c2 100644 --- a/pavelib/utils/db_utils.py +++ b/pavelib/utils/db_utils.py @@ -30,7 +30,7 @@ def remove_files_from_folder(files, folder): continue -def reset_test_db(db_cache_files, update_cache_files=True): +def reset_test_db(db_cache_files, update_cache_files=True, use_existing_db=False): """ Reset the bokchoy test db for a new test run @@ -41,6 +41,8 @@ def reset_test_db(db_cache_files, update_cache_files=True): cmd = '{}/scripts/reset-test-db.sh'.format(Env.REPO_ROOT) if update_cache_files: cmd = '{} --rebuild_cache'.format(cmd) + if use_existing_db: + cmd = '{} --use-existing-db'.format(cmd) sh(cmd) verify_files_exist(db_cache_files) diff --git a/scripts/reset-test-db.sh b/scripts/reset-test-db.sh index 6e23b20ad8..0130ee1f93 100755 --- a/scripts/reset-test-db.sh +++ b/scripts/reset-test-db.sh @@ -46,6 +46,9 @@ for i in "$@"; do -c|--calculate_migrations) CALCULATE_MIGRATIONS=true ;; + -u|--use-existing-db) + USE_EXISTING_DB=true + ;; esac done @@ -94,24 +97,28 @@ rebuild_cache_for_db() { } for db in "${database_order[@]}"; do - echo "CREATE DATABASE IF NOT EXISTS ${databases[$db]};" | mysql $MYSQL_HOST -u root + if ! [[ $USE_EXISTING_DB ]]; then + echo "CREATE DATABASE IF NOT EXISTS ${databases[$db]};" | mysql $MYSQL_HOST -u root - # Clear out the test database - # - # We are using the reset_db command which uses "DROP DATABASE" and - # "CREATE DATABASE" in case the tests are being run in an environment (e.g. devstack - # or a jenkins worker environment) that already ran tests on another commit that had - # different migrations that created, dropped, or altered tables. - echo "Issuing a reset_db command to the $db bok_choy MySQL database." - ./manage.py lms --settings $SETTINGS reset_db --traceback --router $db + # Clear out the test database + # + # We are using the reset_db command which uses "DROP DATABASE" and + # "CREATE DATABASE" in case the tests are being run in an environment (e.g. devstack + # or a jenkins worker environment) that already ran tests on another commit that had + # different migrations that created, dropped, or altered tables. + echo "Issuing a reset_db command to the $db bok_choy MySQL database." + ./manage.py lms --settings $SETTINGS reset_db --traceback --router $db + fi - # If there are cached database schemas/data, then load them. - # If they are missing, then we will want to build new cache files even if - # not explicitly directed to do so via arguments passed to this script. - if [[ ! -f $DB_CACHE_DIR/bok_choy_schema_$db.sql || ! -f $DB_CACHE_DIR/bok_choy_data_$db.json || ! -f $DB_CACHE_DIR/bok_choy_migrations_data_$db.sql ]]; then - REBUILD_CACHE=true - else - load_cache_into_db + if ! [[ $CALCULATE_MIGRATIONS ]]; then + # If there are cached database schemas/data, then load them. + # If they are missing, then we will want to build new cache files even if + # not explicitly directed to do so via arguments passed to this script. + if [[ ! -f $DB_CACHE_DIR/bok_choy_schema_$db.sql || ! -f $DB_CACHE_DIR/bok_choy_data_$db.json || ! -f $DB_CACHE_DIR/bok_choy_migrations_data_$db.sql ]]; then + REBUILD_CACHE=true + else + load_cache_into_db + fi fi done