diff --git a/src/wuttasync/cli/base.py b/src/wuttasync/cli/base.py index 34be0e7..56d6421 100644 --- a/src/wuttasync/cli/base.py +++ b/src/wuttasync/cli/base.py @@ -102,7 +102,7 @@ class ImportCommandHandler(GenericHandler): elif key: self.import_handler = self.app.get_import_handler(key, require=True) - def run(self, ctx, progress=None): # pylint: disable=unused-argument + def run(self, params, progress=None): # pylint: disable=unused-argument """ Run the import/export job(s) based on command line params. @@ -113,29 +113,20 @@ class ImportCommandHandler(GenericHandler): Unless ``--list-models`` was specified on the command line in which case we do :meth:`list_models()` instead. - :param ctx: :class:`typer.Context` instance. + :param params: Dict of params from command line. This must + include a ``'models'`` key, the rest are optional. :param progress: Optional progress indicator factory. """ # maybe just list models and bail - if ctx.params.get("list_models"): - self.list_models(ctx.params) + if params.get("list_models"): + self.list_models(params) return - # otherwise we'll process some data + # otherwise process some data log.debug("using handler: %s", self.import_handler.get_spec()) - - # all params from caller will be passed along - kw = dict(ctx.params) - - # runas user and comment also, but they come from root command - if username := ctx.parent.params.get("runas_username"): - kw["runas_username"] = username - if comment := ctx.parent.params.get("comment"): - kw["transaction_comment"] = comment - - # sort out which models to process + kw = dict(params) models = kw.pop("models") if not models: models = list(self.import_handler.importers) @@ -145,8 +136,6 @@ class ImportCommandHandler(GenericHandler): self.import_handler.get_title(), ", ".join(models), ) - - # process data log.debug("params are: %s", kw) self.import_handler.process_data(*models, **kw) @@ -259,9 +248,7 @@ def import_command_template( # pylint: disable=unused-argument,too-many-argumen warnings_recipients: Annotated[ str, typer.Option( - "--recip", - "--recips", - help="Override the recipient(s) for diff warning email.", + "--recip", help="Override the recipient(s) for diff warning email." ), ] = None, warnings_max_diffs: Annotated[ diff --git a/src/wuttasync/cli/import_csv.py b/src/wuttasync/cli/import_csv.py index 1a55523..4c5694a 100644 --- a/src/wuttasync/cli/import_csv.py +++ b/src/wuttasync/cli/import_csv.py @@ -39,4 +39,4 @@ def import_csv(ctx: typer.Context, **kwargs): # pylint: disable=unused-argument """ config = ctx.parent.wutta_config handler = ImportCommandHandler(config, key="import.to_wutta.from_csv") - handler.run(ctx) + handler.run(ctx.params) diff --git a/src/wuttasync/cli/import_versions.py b/src/wuttasync/cli/import_versions.py index 0057e88..aa82088 100644 --- a/src/wuttasync/cli/import_versions.py +++ b/src/wuttasync/cli/import_versions.py @@ -37,7 +37,14 @@ from .base import import_command, ImportCommandHandler @wutta_typer.command() @import_command -def import_versions(ctx: typer.Context, **kwargs): # pylint: disable=unused-argument +def import_versions( # pylint: disable=unused-argument + ctx: typer.Context, + comment: Annotated[ + str, + typer.Option("--comment", "-m", help="Comment to set on the transaction."), + ] = "import catch-up versions", + **kwargs, +): """ Import latest data to version tables, for Wutta DB """ @@ -63,4 +70,4 @@ def import_versions(ctx: typer.Context, **kwargs): # pylint: disable=unused-arg sys.exit(1) handler = ImportCommandHandler(config, key="import.to_versions.from_wutta") - handler.run(ctx) + handler.run(ctx.params) diff --git a/src/wuttasync/importing/handlers.py b/src/wuttasync/importing/handlers.py index 384fbd3..ac13f28 100644 --- a/src/wuttasync/importing/handlers.py +++ b/src/wuttasync/importing/handlers.py @@ -166,18 +166,6 @@ class ImportHandler(GenericHandler): # pylint: disable=too-many-public-methods See also :attr:`warnings`. """ - runas_username = None - """ - Username responsible for running the import/export job. This is - mostly used for Continuum versioning. - """ - - transaction_comment = None - """ - Optional comment to apply to the transaction, where applicable. - This is mostly used for Continuum versioning. - """ - importers = None """ This should be a dict of all importer/exporter classes available @@ -428,12 +416,6 @@ class ImportHandler(GenericHandler): # pylint: disable=too-many-public-methods if "warnings_max_diffs" in kwargs: self.warnings_max_diffs = kwargs.pop("warnings_max_diffs") - if "runas_username" in kwargs: - self.runas_username = kwargs.pop("runas_username") - - if "transaction_comment" in kwargs: - self.transaction_comment = kwargs.pop("transaction_comment") - return kwargs def begin_transaction(self): @@ -964,41 +946,11 @@ class ToWuttaHandler(ToSqlalchemyHandler): def make_target_session(self): """ - This creates a typical :term:`db session` for the app by - calling - :meth:`~wuttjamaican:wuttjamaican.app.AppHandler.make_session()`. - - It then may "customize" the session slightly. These - customizations only are relevant if Wutta-Continuum versioning - is enabled: - - If :attr:`~ImportHandler.runas_username` is set, the - responsible user (``continuum_user_id``) will be set for the - new session as well. - - Similarly, if :attr:`~ImportHandler.transaction_comment` is - set, it (``continuum_comment``) will also be set for the new - session. + Call + :meth:`~wuttjamaican:wuttjamaican.app.AppHandler.make_session()` + and return the result. :returns: :class:`~wuttjamaican:wuttjamaican.db.sess.Session` instance. """ - model = self.app.model - session = self.app.make_session() - - # set runas user in case continuum versioning is enabled - if self.runas_username: - if user := ( - session.query(model.User) - .filter_by(username=self.runas_username) - .first() - ): - session.info["continuum_user_id"] = user.uuid - else: - log.warning("runas username not found: %s", self.runas_username) - - # set comment in case continuum versioning is enabled - if self.transaction_comment: - session.info["continuum_comment"] = self.transaction_comment - - return session + return self.app.make_session() diff --git a/src/wuttasync/importing/versions.py b/src/wuttasync/importing/versions.py index d558c36..cda77c9 100644 --- a/src/wuttasync/importing/versions.py +++ b/src/wuttasync/importing/versions.py @@ -92,6 +92,13 @@ class FromWuttaToVersions(FromWuttaHandler, ToWuttaHandler): See also :attr:`continuum_uow`. """ + continuum_comment = None + + def consume_kwargs(self, kwargs): + kwargs = super().consume_kwargs(kwargs) + self.continuum_comment = kwargs.pop("comment", None) + return kwargs + def begin_target_transaction(self): # pylint: disable=line-too-long """ @@ -121,8 +128,8 @@ class FromWuttaToVersions(FromWuttaHandler, ToWuttaHandler): self.continuum_txn = self.continuum_uow.create_transaction(self.target_session) - if self.transaction_comment: - self.continuum_txn.meta = {"comment": self.transaction_comment} + if self.continuum_comment: + self.continuum_txn.meta = {"comment": self.continuum_comment} def get_importer_kwargs(self, key, **kwargs): """ diff --git a/tests/cli/test_base.py b/tests/cli/test_base.py index 2370bdd..b8fc954 100644 --- a/tests/cli/test_base.py +++ b/tests/cli/test_base.py @@ -2,7 +2,7 @@ import inspect from unittest import TestCase -from unittest.mock import patch, Mock +from unittest.mock import patch from wuttasync.cli import base as mod from wuttjamaican.testing import DataTestCase @@ -44,28 +44,12 @@ class TestImportCommandHandler(DataTestCase): ) with patch.object(handler, "list_models") as list_models: - ctx = Mock(params={"list_models": True}) - handler.run(ctx) - list_models.assert_called_once_with(ctx.params) - - class Object: - def __init__(self, **kw): - self.__dict__.update(kw) + handler.run({"list_models": True}) + list_models.assert_called_once_with({"list_models": True}) with patch.object(handler, "import_handler") as import_handler: - parent = Mock( - params={ - "runas_username": "fred", - "comment": "hello world", - } - ) - # TODO: why can't we just use Mock here? the parent attr is problematic - ctx = Object(params={"models": []}, parent=parent) - handler.run(ctx) - import_handler.process_data.assert_called_once_with( - runas_username="fred", - transaction_comment="hello world", - ) + handler.run({"models": []}) + import_handler.process_data.assert_called_once_with() def test_list_models(self): handler = self.make_handler( diff --git a/tests/cli/test_import_csv.py b/tests/cli/test_import_csv.py index 2529380..5623176 100644 --- a/tests/cli/test_import_csv.py +++ b/tests/cli/test_import_csv.py @@ -19,4 +19,4 @@ class TestImportCsv(TestCase): ctx = MagicMock(params=params) with patch.object(ImportCommandHandler, "run") as run: mod.import_csv(ctx) - run.assert_called_once_with(ctx) + run.assert_called_once_with(params) diff --git a/tests/cli/test_import_versions.py b/tests/cli/test_import_versions.py index 506a5e9..ea1617d 100644 --- a/tests/cli/test_import_versions.py +++ b/tests/cli/test_import_versions.py @@ -19,4 +19,4 @@ class TestImportCsv(TestCase): ctx = MagicMock(params=params) with patch.object(ImportCommandHandler, "run") as run: mod.import_versions(ctx) - run.assert_called_once_with(ctx) + run.assert_called_once_with(params) diff --git a/tests/importing/test_handlers.py b/tests/importing/test_handlers.py index 21fcaeb..c01b405 100644 --- a/tests/importing/test_handlers.py +++ b/tests/importing/test_handlers.py @@ -190,22 +190,6 @@ class TestImportHandler(DataTestCase): self.assertNotIn("warnings_max_diffs", kw) self.assertEqual(handler.warnings_max_diffs, 30) - # runas_username (consumed) - self.assertIsNone(handler.runas_username) - kw["runas_username"] = "fred" - result = handler.consume_kwargs(kw) - self.assertIs(result, kw) - self.assertNotIn("runas_username", kw) - self.assertEqual(handler.runas_username, "fred") - - # transaction_comment (consumed) - self.assertIsNone(handler.transaction_comment) - kw["transaction_comment"] = "hello world" - result = handler.consume_kwargs(kw) - self.assertIs(result, kw) - self.assertNotIn("transaction_comment", kw) - self.assertEqual(handler.transaction_comment, "hello world") - def test_define_importers(self): handler = self.make_handler() importers = handler.define_importers() @@ -506,41 +490,11 @@ class TestToWuttaHandler(DataTestCase): self.assertEqual(handler.get_target_title(), "what_about_this") def test_make_target_session(self): - model = self.app.model handler = self.make_handler() - fred = model.User(username="fred") - self.session.add(fred) - self.session.commit() - - # makes "new" (mocked in our case) app session, with no runas - # username set by default + # makes "new" (mocked in our case) app session with patch.object(self.app, "make_session") as make_session: make_session.return_value = self.session session = handler.make_target_session() make_session.assert_called_once_with() self.assertIs(session, self.session) - self.assertNotIn("continuum_user_id", session.info) - self.assertNotIn("continuum_user_id", self.session.info) - - # runas user also should not be set, if username is not valid - handler.runas_username = "freddie" - with patch.object(self.app, "make_session") as make_session: - make_session.return_value = self.session - session = handler.make_target_session() - make_session.assert_called_once_with() - self.assertIs(session, self.session) - self.assertNotIn("continuum_user_id", session.info) - self.assertNotIn("continuum_user_id", self.session.info) - - # this time we should have runas user properly set - handler.runas_username = "fred" - with patch.object(self.app, "make_session") as make_session: - make_session.return_value = self.session - session = handler.make_target_session() - make_session.assert_called_once_with() - self.assertIs(session, self.session) - self.assertIn("continuum_user_id", session.info) - self.assertEqual(session.info["continuum_user_id"], fred.uuid) - self.assertIn("continuum_user_id", self.session.info) - self.assertEqual(self.session.info["continuum_user_id"], fred.uuid) diff --git a/tests/importing/test_versions.py b/tests/importing/test_versions.py index eea6171..2cd4ec0 100644 --- a/tests/importing/test_versions.py +++ b/tests/importing/test_versions.py @@ -14,6 +14,20 @@ class TestFromWuttaToVersions(VersionTestCase): def make_handler(self, **kwargs): return mod.FromWuttaToVersions(self.config, **kwargs) + def test_consume_kwargs(self): + + # no comment by default + handler = self.make_handler() + kw = handler.consume_kwargs({}) + self.assertEqual(kw, {}) + self.assertIsNone(handler.continuum_comment) + + # but can provide one + handler = self.make_handler() + kw = handler.consume_kwargs({"comment": "yeehaw"}) + self.assertEqual(kw, {}) + self.assertEqual(handler.continuum_comment, "yeehaw") + def test_begin_target_transaction(self): model = self.app.model txncls = continuum.transaction_class(model.User) @@ -30,7 +44,7 @@ class TestFromWuttaToVersions(VersionTestCase): # with comment handler = self.make_handler() - handler.transaction_comment = "yeehaw" + handler.continuum_comment = "yeehaw" handler.begin_target_transaction() self.assertIn("comment", handler.continuum_txn.meta) self.assertEqual(handler.continuum_txn.meta["comment"], "yeehaw")