From 70e780d1eb48562bb9f03bae3d2759d5961c91e9 Mon Sep 17 00:00:00 2001 From: Javier Date: Tue, 8 Mar 2022 11:26:30 -0300 Subject: [PATCH 1/3] fix(models): add oid field to detection and non detection --- db_plugins/db/mongo/models.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/db_plugins/db/mongo/models.py b/db_plugins/db/mongo/models.py index 1919154..ed0e6ab 100644 --- a/db_plugins/db/mongo/models.py +++ b/db_plugins/db/mongo/models.py @@ -64,6 +64,7 @@ class Detection(Base, generic_models.Detection): Field() ) # Telescope id (this gives the spatial coordinates of the observatory, e.g. ZTF, ATLAS-HKO, ATLAS-MLO) aid = Field() + oid = Field() candid = Field() mjd = Field() fid = Field() @@ -90,6 +91,7 @@ class NonDetection(Base, generic_models.NonDetection): aid = Field() tid = Field() + oid = Field() mjd = Field() diffmaglim = Field() fid = Field() From 1374ec0b8046a7090272ff99f4c3ebfda05740de Mon Sep 17 00:00:00 2001 From: Javier Date: Tue, 8 Mar 2022 15:29:48 -0300 Subject: [PATCH 2/3] dev(black): add gh actions and formatted code --- .github/workflows/black.yml | 13 +++++++++++++ .github/workflows/unittests.yml | 6 ++---- db_plugins/cli/manage.py | 12 +++--------- db_plugins/db/mongo/connection.py | 2 +- db_plugins/db/mongo/query.py | 12 +++--------- db_plugins/db/sql/migrations/env.py | 8 +++----- 6 files changed, 25 insertions(+), 28 deletions(-) create mode 100644 .github/workflows/black.yml diff --git a/.github/workflows/black.yml b/.github/workflows/black.yml new file mode 100644 index 0000000..cb168d1 --- /dev/null +++ b/.github/workflows/black.yml @@ -0,0 +1,13 @@ +name: Lint + +on: [push, pull_request] + +jobs: + lint: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - uses: psf/black@stable + with: + options: "--check --verbose" + src: "db_plugins/" \ No newline at end of file diff --git a/.github/workflows/unittests.yml b/.github/workflows/unittests.yml index 6b94115..69c421f 100644 --- a/.github/workflows/unittests.yml +++ b/.github/workflows/unittests.yml @@ -1,4 +1,4 @@ -# This workflow will install Python dependencies, run tests and lint with a single version of Python +# This workflow will install Python dependencies and run tests # For more information see: https://help.github.com/actions/language-and-framework-guides/using-python-with-github-actions name: UnitTests @@ -21,10 +21,8 @@ jobs: - name: Install dependencies run: | python -m pip install --upgrade pip - pip install black pytest coverage alchemy_mock mongomock + pip install pytest coverage alchemy_mock mongomock if [ -f requirements.txt ]; then pip install -r requirements.txt; fi - - name: Black Code Formatter - uses: lgeiger/black-action@v1.0.1 - name: Test with pytest run: | coverage run --source db_plugins --omit db_plugins/db/sql/serializers.py -m pytest tests/unittest/ diff --git a/db_plugins/cli/manage.py b/db_plugins/cli/manage.py index 0393576..f034bf0 100644 --- a/db_plugins/cli/manage.py +++ b/db_plugins/cli/manage.py @@ -29,15 +29,11 @@ def initdb(settings_path, db=None): del sys.modules["settings"] if "SQL" in DB_CONFIG: init_sql(DB_CONFIG["SQL"], db) - click.echo( - "Database created with credentials from {}".format(settings_path) - ) + click.echo("Database created with credentials from {}".format(settings_path)) elif "MONGO" in DB_CONFIG: init_mongo(DB_CONFIG["MONGO"], db) - click.echo( - "Database created with credentials from {}".format(settings_path) - ) + click.echo("Database created with credentials from {}".format(settings_path)) else: raise Exception("Invalid settings file") @@ -73,9 +69,7 @@ def migrate(settings_path): sys.path.pop(-1) if "SQL" in DB_CONFIG: migrate_sql() - click.echo( - "Migrated database with config from {}".format(settings_path) - ) + click.echo("Migrated database with config from {}".format(settings_path)) else: print("ERROR", DB_CONFIG) diff --git a/db_plugins/db/mongo/connection.py b/db_plugins/db/mongo/connection.py index f9fb14c..0a3436d 100644 --- a/db_plugins/db/mongo/connection.py +++ b/db_plugins/db/mongo/connection.py @@ -54,7 +54,7 @@ def connect(self, config): username=config["USER"], password=config["PASSWORD"], port=config["PORT"], - authSource=config["DATABASE"] + authSource=config["DATABASE"], ) self.base.set_database(config["DATABASE"]) self.database = self.client[config["DATABASE"]] diff --git a/db_plugins/db/mongo/query.py b/db_plugins/db/mongo/query.py index e7291b7..018f818 100644 --- a/db_plugins/db/mongo/query.py +++ b/db_plugins/db/mongo/query.py @@ -136,9 +136,7 @@ def update(self, instance, args): def bulk_update_creator(collection_class): - def bulk_update( - self, instances: list, args: list, filter_fields: list = [] - ): + def bulk_update(self, instances: list, args: list, filter_fields: list = []): model = type(instances[0]) self.init_collection(model) requests = [] @@ -149,9 +147,7 @@ def bulk_update( {"$set": args[i]}, ) ) - return collection_class.bulk_write( - self, requests=requests, ordered=False - ) + return collection_class.bulk_write(self, requests=requests, ordered=False) return bulk_update @@ -242,9 +238,7 @@ def find_one(self, filter_by={}, model: Base = None): def find_all_creator(collection_class): - def find_all( - self, model: Base = None, filter_by={}, paginate=True, **kwargs - ): + def find_all(self, model: Base = None, filter_by={}, paginate=True, **kwargs): """Find list of items of the specified model. If there are too many items a timeout can happen. diff --git a/db_plugins/db/sql/migrations/env.py b/db_plugins/db/sql/migrations/env.py index 47e2515..2e24efd 100644 --- a/db_plugins/db/sql/migrations/env.py +++ b/db_plugins/db/sql/migrations/env.py @@ -21,7 +21,7 @@ db_credentials = db_config["SQLALCHEMY_DATABASE_URL"] -config.set_main_option('sqlalchemy.url', db_credentials) +config.set_main_option("sqlalchemy.url", db_credentials) # Interpret the config file for Python logging. # This line sets up loggers basically. fileConfig(config.config_file_name) @@ -29,7 +29,7 @@ # add your model's MetaData object here # for 'autogenerate' support target_metadata = Base.metadata -#target_metadata = None +# target_metadata = None # other values from the config, defined by the needs of env.py, # can be acquired: @@ -75,9 +75,7 @@ def run_migrations_online(): ) with connectable.connect() as connection: - context.configure( - connection=connection, target_metadata=target_metadata - ) + context.configure(connection=connection, target_metadata=target_metadata) with context.begin_transaction(): context.run_migrations() From 9ceb72ef22648bd1b7bba72f148c6ba4dbe29f96 Mon Sep 17 00:00:00 2001 From: Javier Date: Tue, 8 Mar 2022 15:54:13 -0300 Subject: [PATCH 3/3] test(unittest): add oid to detection and non detections on test --- tests/integration/integration_test.py | 3 -- tests/unittest/cli/test_manage.py | 34 ++++--------- tests/unittest/db/test_mongo.py | 8 +-- tests/unittest/db/test_mongo_models.py | 70 ++++++++++++++------------ 4 files changed, 48 insertions(+), 67 deletions(-) diff --git a/tests/integration/integration_test.py b/tests/integration/integration_test.py index c03ffb6..06f839a 100644 --- a/tests/integration/integration_test.py +++ b/tests/integration/integration_test.py @@ -9,9 +9,6 @@ from db_plugins.db.generic import Pagination from sqlalchemy.engine.reflection import Inspector import unittest -import json -import time -import datetime class SQLConnectionTest(unittest.TestCase): diff --git a/tests/unittest/cli/test_manage.py b/tests/unittest/cli/test_manage.py index 7239724..8e10a45 100644 --- a/tests/unittest/cli/test_manage.py +++ b/tests/unittest/cli/test_manage.py @@ -21,9 +21,7 @@ def test_init_sql(self, main_mock, mock_makedirs, mock_connection): @mock.patch("db_plugins.cli.manage.init_sql") def test_initdb_sql(self, mock_init_sql): - with self.runner.isolated_filesystem( - temp_dir=self.settings_path - ) as td: + with self.runner.isolated_filesystem(temp_dir=self.settings_path) as td: with open("settings.py", "w") as f: f.write("DB_CONFIG=") DB_CONFIG = { @@ -39,8 +37,7 @@ def test_initdb_sql(self, mock_init_sql): assert result.exit_code == 0 mock_init_sql.assert_called() assert ( - "Database created with credentials from {}".format(td) - in result.output + "Database created with credentials from {}".format(td) in result.output ) def test_initdb_error(self): @@ -50,42 +47,32 @@ def test_initdb_error(self): @mock.patch("db_plugins.db.sql.initialization.alembic.config.main") def test_make_migrations(self, main_mock): - with self.runner.isolated_filesystem( - temp_dir=self.settings_path - ) as td: + with self.runner.isolated_filesystem(temp_dir=self.settings_path) as td: with open("settings.py", "w") as f: f.write("DB_CONFIG=") DB_CONFIG = { "SQL": {"SQLALCHEMY_DATABASE_URL": "sqlite:///:memory:"}, } f.write(str(DB_CONFIG)) - result = self.runner.invoke( - manage.make_migrations, ["--settings_path", td] - ) + result = self.runner.invoke(manage.make_migrations, ["--settings_path", td]) assert result.exit_code == 0 main_mock.assert_called() def test_make_migrations_error(self): - result = self.runner.invoke( - manage.make_migrations, "--settings_path fail" - ) + result = self.runner.invoke(manage.make_migrations, "--settings_path fail") assert result.exit_code != 0 assert "Settings file not found" == str(result.exception) @mock.patch("db_plugins.db.sql.initialization.alembic.config.main") def test_migrate(self, main_mock): - with self.runner.isolated_filesystem( - temp_dir=self.settings_path - ) as td: + with self.runner.isolated_filesystem(temp_dir=self.settings_path) as td: with open("settings.py", "w") as f: f.write("DB_CONFIG=") DB_CONFIG = { "SQL": {"SQLALCHEMY_DATABASE_URL": "sqlite:///:memory:"}, } f.write(str(DB_CONFIG)) - result = self.runner.invoke( - manage.migrate, ["--settings_path", td] - ) + result = self.runner.invoke(manage.migrate, ["--settings_path", td]) assert result.exit_code == 0 main_mock.assert_called() @@ -110,9 +97,7 @@ def test_init_mongo(self, mock_connection): @mock.patch("db_plugins.cli.manage.init_mongo") def test_initdb_mongo(self, mock_init_mongo): - with self.runner.isolated_filesystem( - temp_dir=self.settings_path - ) as td: + with self.runner.isolated_filesystem(temp_dir=self.settings_path) as td: with open("settings.py", "w") as f: f.write("DB_CONFIG=") DB_CONFIG = { @@ -129,6 +114,5 @@ def test_initdb_mongo(self, mock_init_mongo): assert result.exit_code == 0 mock_init_mongo.assert_called() assert ( - "Database created with credentials from {}".format(td) - in result.output + "Database created with credentials from {}".format(td) in result.output ) diff --git a/tests/unittest/db/test_mongo.py b/tests/unittest/db/test_mongo.py index 0374a77..85cf03a 100644 --- a/tests/unittest/db/test_mongo.py +++ b/tests/unittest/db/test_mongo.py @@ -37,9 +37,7 @@ def test_connect(self): def test_create_db(self): self.conn.create_db() - collections = self.client[ - self.config["DATABASE"] - ].list_collection_names() + collections = self.client[self.config["DATABASE"]].list_collection_names() expected = ["object", "detection", "non_detection"] self.assertEqual(collections, expected) @@ -94,9 +92,7 @@ def setUp(self): self.database = client["database"] self.obj_collection = self.database["object"] self.obj_collection.insert_one({"test": "test"}) - self.mongo_query_class = mongo_query_creator( - mongomock.collection.Collection - ) + self.mongo_query_class = mongo_query_creator(mongomock.collection.Collection) self.query = self.mongo_query_class( model=Object, database=self.database, diff --git a/tests/unittest/db/test_mongo_models.py b/tests/unittest/db/test_mongo_models.py index 0763740..c328f47 100644 --- a/tests/unittest/db/test_mongo_models.py +++ b/tests/unittest/db/test_mongo_models.py @@ -11,13 +11,11 @@ def test_object_creates(self): firstmjd="firstmjd", meanra=100.0, meandec=50.0, - ndet="ndet" + ndet="ndet", ) self.assertIsInstance(o, models.Object) self.assertIsInstance(o, dict) - self.assertEqual( - o["loc"], {"type": "Point", "coordinates": [-80.0, 50]} - ) + self.assertEqual(o["loc"], {"type": "Point", "coordinates": [-80.0, 50]}) self.assertEqual( o._meta.tablename, models.Object.__tablename__, @@ -56,6 +54,7 @@ def test_detection_creates(self): d = models.Detection( tid="tid", aid="aid", + oid="oid", candid="candid", mjd="mjd", fid="fid", @@ -64,19 +63,19 @@ def test_detection_creates(self): rb="rb", mag="mag", e_mag="e_mag", - rfid = "rfid", - e_ra = "e_ra", - e_dec = "e_dec", - isdiffpos = "isdiffpos", - magpsf_corr = "magpsf_corr", - sigmapsf_corr = "sigmapsf_corr", - sigmapsf_corr_ext = "sigmapsf_corr_ext", - corrected = "corrected", - dubious = "dubious", - parent_candid = "parent_candid", - has_stamp = "has_stamp", - step_id_corr = "step_id_corr", - rbversion = "rbversion", + rfid="rfid", + e_ra="e_ra", + e_dec="e_dec", + isdiffpos="isdiffpos", + magpsf_corr="magpsf_corr", + sigmapsf_corr="sigmapsf_corr", + sigmapsf_corr_ext="sigmapsf_corr_ext", + corrected="corrected", + dubious="dubious", + parent_candid="parent_candid", + has_stamp="has_stamp", + step_id_corr="step_id_corr", + rbversion="rbversion", ) self.assertIsInstance(d, models.Detection) self.assertIsInstance(d, dict) @@ -91,6 +90,7 @@ def test_detection_with_extra_fields(self): o = models.Detection( tid="tid", aid="aid", + oid="oid", candid="candid", mjd="mjd", fid="fid", @@ -100,20 +100,21 @@ def test_detection_with_extra_fields(self): mag="mag", e_mag="e_mag", rfid="rfid", - e_ra = "e_ra", - e_dec = "e_dec", - isdiffpos = "isdiffpos", - corrected = "corrected", - parent_candid = "parent_candid", - has_stamp = "has_stamp", - step_id_corr = "step_id_corr", - rbversion = "rbversion", + e_ra="e_ra", + e_dec="e_dec", + isdiffpos="isdiffpos", + corrected="corrected", + parent_candid="parent_candid", + has_stamp="has_stamp", + step_id_corr="step_id_corr", + rbversion="rbversion", extra="extra", ) self.assertEqual(o["extra_fields"], {"extra": "extra"}) o = models.Detection( tid="tid", aid="aid", + oid="oid", candid="candid", mjd="mjd", fid="fid", @@ -123,14 +124,14 @@ def test_detection_with_extra_fields(self): mag="mag", e_mag="e_mag", rfid="rfid", - e_ra = "e_ra", - e_dec = "e_dec", - isdiffpos = "isdiffpos", - corrected = "corrected", - parent_candid = "parent_candid", - has_stamp = "has_stamp", - step_id_corr = "step_id_corr", - rbversion = "rbversion", + e_ra="e_ra", + e_dec="e_dec", + isdiffpos="isdiffpos", + corrected="corrected", + parent_candid="parent_candid", + has_stamp="has_stamp", + step_id_corr="step_id_corr", + rbversion="rbversion", extra_fields={"extra": "extra"}, ) self.assertEqual(o["extra_fields"], {"extra": "extra"}) @@ -138,6 +139,7 @@ def test_detection_with_extra_fields(self): def test_non_detection_creates(self): o = models.NonDetection( aid="aid", + oid="oid", tid="sid", mjd="mjd", diffmaglim="diffmaglim", @@ -155,6 +157,7 @@ def test_non_detection_fails_creation(self): def test_non_detection_with_extra_fields(self): o = models.NonDetection( aid="aid", + oid="oid", tid="tid", mjd="mjd", diffmaglim="diffmaglim", @@ -164,6 +167,7 @@ def test_non_detection_with_extra_fields(self): self.assertEqual(o["extra_fields"], {"extra": "extra"}) o = models.NonDetection( aid="aid", + oid="oid", tid="tid", mjd="mjd", diffmaglim="diffmaglim",