From 106ed7b54a0e2afd7ae7565e4265c707eee92e8f Mon Sep 17 00:00:00 2001 From: Joel Davies Date: Fri, 13 Sep 2024 13:22:48 +0000 Subject: [PATCH 1/4] Add ims-migration script and an example migration script #339 --- Dockerfile | 3 + Dockerfile.prod | 3 + .../migrations/__init__.py | 0 .../migrations/migration.py | 160 ++++++++++++++++++ .../migrations/scripts/__init__.py | 0 .../migrations/scripts/example_migration.py | 58 +++++++ pyproject.toml | 3 + 7 files changed, 227 insertions(+) create mode 100644 inventory_management_system_api/migrations/__init__.py create mode 100644 inventory_management_system_api/migrations/migration.py create mode 100644 inventory_management_system_api/migrations/scripts/__init__.py create mode 100644 inventory_management_system_api/migrations/scripts/example_migration.py diff --git a/Dockerfile b/Dockerfile index bc1edce4..b6a30ef2 100644 --- a/Dockerfile +++ b/Dockerfile @@ -2,6 +2,9 @@ FROM python:3.12.5-alpine3.20@sha256:bb5d0ac04679d78a1258e7dfacdb4d9bdefe9a10480 WORKDIR /inventory-management-system-api-run +# Requirement when using a different workdir to get scripts to import correctly +ENV PYTHONPATH="${PYTHONPATH}:/inventory-management-system-api-run" + COPY pyproject.toml ./ COPY inventory_management_system_api/ inventory_management_system_api/ diff --git a/Dockerfile.prod b/Dockerfile.prod index 2433f778..ac6acd28 100644 --- a/Dockerfile.prod +++ b/Dockerfile.prod @@ -2,6 +2,9 @@ FROM python:3.12.5-alpine3.20@sha256:bb5d0ac04679d78a1258e7dfacdb4d9bdefe9a10480 WORKDIR /inventory-management-system-api-run +# Requirement when using a different workdir to get scripts to import correctly +ENV PYTHONPATH="${PYTHONPATH}:/inventory-management-system-api-run" + COPY README.md pyproject.toml ./ # Copy inventory_management_system_api source files COPY inventory_management_system_api/ inventory_management_system_api/ diff --git a/inventory_management_system_api/migrations/__init__.py b/inventory_management_system_api/migrations/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/inventory_management_system_api/migrations/migration.py b/inventory_management_system_api/migrations/migration.py new file mode 100644 index 00000000..3b72a387 --- /dev/null +++ b/inventory_management_system_api/migrations/migration.py @@ -0,0 +1,160 @@ +"""Module for providing a migration script""" + +import argparse +import importlib +import logging +from abc import ABC, abstractmethod + +from pymongo.client_session import ClientSession +from pymongo.database import Database + +from inventory_management_system_api.core.database import get_database, mongodb_client + +logger = logging.getLogger() + + +class BaseMigration(ABC): + """Base class for a migration with a forward and backward step""" + + @abstractmethod + def __init__(self, database: Database): + pass + + @property + @abstractmethod + def description(self) -> str: + """Description of this migration""" + return "" + + @abstractmethod + def forward(self, session: ClientSession): + """Method for executing the migration""" + + def forward_after_transaction(self, session: ClientSession): + """Method called after the forward function is called to do anything that can't be done inside a transaction + (ONLY USE IF NECESSARY e.g. dropping a collection)""" + + @abstractmethod + def backward(self, session: ClientSession): + """Method for reversing the migration""" + + def backward_after_transaction(self, session: ClientSession): + """Method called after the backward function is called to do anything that can't be done inside a transaction + (ONLY USE IF NECESSARY e.g. dropping a collection)""" + + +class SubCommand(ABC): + """Base class for a sub command""" + + def __init__(self, help_message: str): + self.help_message = help_message + + @abstractmethod + def setup(self, parser: argparse.ArgumentParser): + """Setup the parser by adding any parameters here""" + + @abstractmethod + def run(self, args: argparse.Namespace): + """Run the command with the given parameters as added by 'setup'""" + + +def load_migration(name: str) -> BaseMigration: + """Loads a migration script from the scripts module""" + + migration_module = importlib.import_module(f"inventory_management_system_api.migrations.scripts.{name}") + migration_class = getattr(migration_module, "Migration", None) + + database = get_database() + return migration_class(database) + + +class CommandList(SubCommand): + """Command that lists available database migrations""" + + def __init__(self): + super().__init__(help_message="List all available database migrations") + + def setup(self, parser: argparse.ArgumentParser): + pass + + def run(self, args: argparse.Namespace): + # Find a list of all available migration scripts + with importlib.resources.path("inventory_management_system_api.migrations.scripts", "") as scripts_path: + files_in_scripts = list(scripts_path.iterdir()) + available_migrations = list( + filter(lambda name: "__" not in name, [file.name.replace(".py", "") for file in files_in_scripts]) + ) + for migration_name in available_migrations: + migration = load_migration(migration_name) + + print(f"{migration_name} - {migration.description}") + + +class CommandForward(SubCommand): + """Command that performs a forward database migration""" + + def __init__(self): + super().__init__(help_message="Performs a forward database migration") + + def setup(self, parser: argparse.ArgumentParser): + parser.add_argument("migration", help_message="Name of the migration to perform") + + def run(self, args: argparse.Namespace): + migration_instance: BaseMigration = load_migration(args.migration) + + # Run migration inside a session to lock writes and revert the changes if it fails + with mongodb_client.start_session() as session: + with session.start_transaction(): + logger.info("Performing forward migration...") + migration_instance.forward(session) + # Run some things outside the transaction e.g. if needing to drop a collection + migration_instance.forward_after_transaction(session) + logger.info("Done!") + + +class CommandBackward(SubCommand): + """Command that performs a backward database migration""" + + def __init__(self): + super().__init__(help_message="Performs a backward database migration") + + def setup(self, parser: argparse.ArgumentParser): + parser.add_argument("migration", help_message="Name of the migration to revert") + + def run(self, args: argparse.Namespace): + migration_instance: BaseMigration = load_migration(args.migration) + + # Run migration inside a session to lock writes and revert the changes if it fails + with mongodb_client.start_session() as session: + with session.start_transaction(): + logger.info("Performing backward migration...") + migration_instance.backward(session) + # Run some things outside the transaction e.g. if needing to drop a collection + migration_instance.backward_after_transaction(session) + logger.info("Done!") + + +# List of subcommands +commands: dict[str, SubCommand] = { + "list": CommandList(), + "forward": CommandForward(), + "backward": CommandBackward(), +} + + +def main(): + """Entrypoint for the ims-migrate script""" + + parser = argparse.ArgumentParser() + + subparser = parser.add_subparsers(dest="command") + + for command_name, command in commands.items(): + command_parser = subparser.add_parser(command_name, help=command.help_message) + command.setup(command_parser) + + args = parser.parse_args() + + logging.basicConfig(level=logging.INFO) + + commands[args.command].run(args) diff --git a/inventory_management_system_api/migrations/scripts/__init__.py b/inventory_management_system_api/migrations/scripts/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/inventory_management_system_api/migrations/scripts/example_migration.py b/inventory_management_system_api/migrations/scripts/example_migration.py new file mode 100644 index 00000000..fd9fab35 --- /dev/null +++ b/inventory_management_system_api/migrations/scripts/example_migration.py @@ -0,0 +1,58 @@ +""" +Module providing an example migration that does nothing +""" + +from typing import Collection + +from pymongo.client_session import ClientSession +from pymongo.database import Database + +from inventory_management_system_api.migrations.migration import BaseMigration + +# When the migration will modify database models by adding data may be a good idea to put the old here and pass any data +# between them and the new ones before updating them in the database, to ensure all modifications are as expected +# e.g. + + +# class OldUnit(BaseModel): +# """ +# Old database model for a Unit +# """ + +# value: str +# code: str + + +class Migration(BaseMigration): + """Example migration that does nothing""" + + description = "Example migration that does nothing" + + def __init__(self, database: Database): + """Obtain any collections required for the migration here e.g.""" + + self._units_collection: Collection = database.units + + def forward(self, session: ClientSession): + """This function should actually perform the migration + + All database functions should be given the session in order to ensure all updates are done within a transaction + """ + + # Perform any database updates here e.g. for renaming a field + + # self._units_collection.update_many( + # {}, {"$rename": {"value": "renamed_value"}}, session=session + # ) + + def backward(self, session: ClientSession): + """This function should reverse the migration + + All database functions should be given the session in order to ensure all updates are done within a transaction + """ + + # Perform any database updates here e.g. for renaming a field + + # self._units_collection.update_many( + # {}, {"$rename": {"renamed_value": "value"}}, session=session + # ) diff --git a/pyproject.toml b/pyproject.toml index dadab49a..7277f360 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -16,6 +16,9 @@ dependencies = [ [project.urls] "Repository" = "https://github.com/ral-facilities/inventory-management-system-api" +[project.scripts] +"ims-migrate" = "inventory_management_system_api.migrations.migration:main" + [project.optional-dependencies] code-analysis = [ "pylint==3.2.5", From 42a73ce4e07ba6ac195a591f65dddef6edbddd95 Mon Sep 17 00:00:00 2001 From: Joel Davies Date: Fri, 13 Sep 2024 13:32:25 +0000 Subject: [PATCH 2/4] Add instructions to README #339 --- README.md | 32 ++++++++++++++++++- .../migrations/migration.py | 4 +-- .../migrations/scripts/example_migration.py | 7 ++++ 3 files changed, 40 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 069fd851..3d6f8756 100644 --- a/README.md +++ b/README.md @@ -294,7 +294,7 @@ values loaded from the `.env` file. Listed below are the environment variables supported by the application. | Environment Variable | Description | Mandatory | Default Value | -|-----------------------------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|---------------------|-------------------------------------------------------| +| --------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------- | ----------------------------------------------------- | | `API__TITLE` | The title of the API which is added to the generated OpenAPI. | No | `Inventory Management System API` | | `API__DESCRIPTION` | The description of the API which is added to the generated OpenAPI. | No | `This is the API for the Inventory Management System` | | `API__ROOT_PATH` | (If using a proxy) The path prefix handled by a proxy that is not seen by the app. | No | ` ` | @@ -320,3 +320,33 @@ encoding the token. Once the JWT access token is decoded successfully, it checks payload, and it has not expired. This means that any microservice can be used to generate JWT access tokens so long as it meets the above criteria. The [LDAP-JWT Authentication Service](https://github.com/ral-facilities/ldap-jwt-auth) is a microservice that provides user authentication against an LDAP server and returns a JWT access token. + +### Migrations + +Migration scripts are located inside the `inventory_management_system/migrations/scripts`. See the +`example_migration.py` for an example on how to implement one. Any new migrations added should be automatically picked +up and shown via + +```bash +ims-migration list +``` + +or + +```bash +docker exec -it inventory_management_system_api_container ims-migrate list +``` + +if running in Docker. + +To perform a migration you should use + +```bash +ims-migration forward +``` + +To revert the same migration use + +```bash +ims-migration backward +``` diff --git a/inventory_management_system_api/migrations/migration.py b/inventory_management_system_api/migrations/migration.py index 3b72a387..f28a391f 100644 --- a/inventory_management_system_api/migrations/migration.py +++ b/inventory_management_system_api/migrations/migration.py @@ -97,7 +97,7 @@ def __init__(self): super().__init__(help_message="Performs a forward database migration") def setup(self, parser: argparse.ArgumentParser): - parser.add_argument("migration", help_message="Name of the migration to perform") + parser.add_argument("migration", help="Name of the migration to perform") def run(self, args: argparse.Namespace): migration_instance: BaseMigration = load_migration(args.migration) @@ -119,7 +119,7 @@ def __init__(self): super().__init__(help_message="Performs a backward database migration") def setup(self, parser: argparse.ArgumentParser): - parser.add_argument("migration", help_message="Name of the migration to revert") + parser.add_argument("migration", help="Name of the migration to revert") def run(self, args: argparse.Namespace): migration_instance: BaseMigration = load_migration(args.migration) diff --git a/inventory_management_system_api/migrations/scripts/example_migration.py b/inventory_management_system_api/migrations/scripts/example_migration.py index fd9fab35..aec88525 100644 --- a/inventory_management_system_api/migrations/scripts/example_migration.py +++ b/inventory_management_system_api/migrations/scripts/example_migration.py @@ -2,6 +2,7 @@ Module providing an example migration that does nothing """ +import logging from typing import Collection from pymongo.client_session import ClientSession @@ -9,6 +10,8 @@ from inventory_management_system_api.migrations.migration import BaseMigration +logger = logging.getLogger() + # When the migration will modify database models by adding data may be a good idea to put the old here and pass any data # between them and the new ones before updating them in the database, to ensure all modifications are as expected # e.g. @@ -45,6 +48,8 @@ def forward(self, session: ClientSession): # {}, {"$rename": {"value": "renamed_value"}}, session=session # ) + logger.info("example_migration forward migration (that does nothing)") + def backward(self, session: ClientSession): """This function should reverse the migration @@ -56,3 +61,5 @@ def backward(self, session: ClientSession): # self._units_collection.update_many( # {}, {"$rename": {"renamed_value": "value"}}, session=session # ) + + logger.info("example_migration backward migration (that does nothing)") From e2c291002f8aa6b74bece6819ce1a7ed53cbe30a Mon Sep 17 00:00:00 2001 From: Joel Davies Date: Fri, 13 Sep 2024 13:51:54 +0000 Subject: [PATCH 3/4] Apply similar changes to dev_cli.py #339 --- scripts/dev_cli.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/scripts/dev_cli.py b/scripts/dev_cli.py index d00c05e8..b6a5611c 100644 --- a/scripts/dev_cli.py +++ b/scripts/dev_cli.py @@ -63,6 +63,7 @@ def run_mongodb_command(args: list[str], stdin: Optional[TextIOWrapper] = None, def add_mongodb_auth_args(parser: argparse.ArgumentParser): """Adds common arguments for MongoDB authentication""" + parser.add_argument("-u", "--username", default="root", help="Username for MongoDB authentication") parser.add_argument("-p", "--password", default="example", help="Password for MongoDB authentication") @@ -81,8 +82,8 @@ def get_mongodb_auth_args(args: argparse.Namespace): class SubCommand(ABC): """Base class for a sub command""" - def __init__(self, help: str): - self.help = help + def __init__(self, help_message: str): + self.help_message = help_message @abstractmethod def setup(self, parser: argparse.ArgumentParser): @@ -103,7 +104,7 @@ class CommandDBInit(SubCommand): """ def __init__(self): - super().__init__(help="Initialise database for development (using docker on linux)") + super().__init__(help_message="Initialise database for development (using docker on linux)") def setup(self, parser: argparse.ArgumentParser): add_mongodb_auth_args(parser) @@ -168,7 +169,7 @@ class CommandDBImport(SubCommand): """Command that imports mock data into the database""" def __init__(self): - super().__init__(help="Imports database for development") + super().__init__(help_message="Imports database for development") def setup(self, parser: argparse.ArgumentParser): add_mongodb_auth_args(parser) @@ -198,7 +199,7 @@ class CommandDBGenerate(SubCommand): """ def __init__(self): - super().__init__(help="Generates new test data for the database and dumps it") + super().__init__(help_message="Generates new test data for the database and dumps it") def setup(self, parser: argparse.ArgumentParser): add_mongodb_auth_args(parser) @@ -272,7 +273,7 @@ def main(): subparser = parser.add_subparsers(dest="command") for command_name, command in commands.items(): - command_parser = subparser.add_parser(command_name, help=command.help) + command_parser = subparser.add_parser(command_name, help=command.help_message) command.setup(command_parser) args = parser.parse_args() From a312b0446607ffcd3d7f9f827da1cbae8e9b7b0c Mon Sep 17 00:00:00 2001 From: Joel Davies Date: Fri, 13 Sep 2024 14:02:14 +0000 Subject: [PATCH 4/4] Fix typos in README #339 --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 3d6f8756..ae66f608 100644 --- a/README.md +++ b/README.md @@ -328,7 +328,7 @@ Migration scripts are located inside the `inventory_management_system/migrations up and shown via ```bash -ims-migration list +ims-migrate list ``` or @@ -342,11 +342,11 @@ if running in Docker. To perform a migration you should use ```bash -ims-migration forward +ims-migrate forward ``` To revert the same migration use ```bash -ims-migration backward +ims-migrate backward ```