diff --git a/planemo/database/factory.py b/planemo/database/factory.py index 0250edf53..571ec4751 100644 --- a/planemo/database/factory.py +++ b/planemo/database/factory.py @@ -1,5 +1,7 @@ """Create a DatabaseSource from supplied planemo configuration.""" +from typing import Optional + from galaxy.util.commands import which from .interface import DatabaseSource @@ -8,7 +10,7 @@ from .postgres_singularity import SingularityPostgresDatabaseSource -def create_database_source(**kwds) -> DatabaseSource: +def create_database_source(profile_directory: Optional[str] = None, **kwds) -> DatabaseSource: """Return a :class:`planemo.database.interface.DatabaseSource` for configuration.""" database_type = kwds.get("database_type", "auto") if database_type == "auto": @@ -26,7 +28,7 @@ def create_database_source(**kwds) -> DatabaseSource: elif database_type == "postgres_docker": return DockerPostgresDatabaseSource(**kwds) elif database_type == "postgres_singularity": - return SingularityPostgresDatabaseSource(**kwds) + return SingularityPostgresDatabaseSource(profile_directory=profile_directory, **kwds) # TODO # from .sqlite import SqliteDatabaseSource # elif database_type == "sqlite": diff --git a/planemo/database/interface.py b/planemo/database/interface.py index ffd3c1654..a3827dfb1 100644 --- a/planemo/database/interface.py +++ b/planemo/database/interface.py @@ -1,18 +1,12 @@ """Describe the interface classes of the planemo.database package.""" import abc +from typing import Optional class DatabaseSource(metaclass=abc.ABCMeta): """Interface describing a source of profile databases.""" - @abc.abstractmethod - def create_database(self, identifier): - """Create a database with specified short identifier. - - Throw an exception if it already exists. - """ - @abc.abstractmethod def delete_database(self, identifier): """Delete a database with specified short identifier. @@ -25,8 +19,16 @@ def list_databases(self): """Return identifiers associated with database source.""" @abc.abstractmethod - def sqlalchemy_url(self, identifier): + def sqlalchemy_url(self, identifier) -> Optional[str]: """Return a URL string for use by sqlalchemy.""" + def start(self): + """Start the database source, if necessary.""" + pass + + def stop(self): + """Stop the database source, if necessary.""" + pass + __all__ = ("DatabaseSource",) diff --git a/planemo/database/postgres_docker.py b/planemo/database/postgres_docker.py index 84cbed038..33b3b406b 100644 --- a/planemo/database/postgres_docker.py +++ b/planemo/database/postgres_docker.py @@ -63,11 +63,25 @@ def __init__(self, **kwds): self.database_port = DEFAULT_POSTGRES_PORT_EXPOSE self._kwds = kwds self._docker_host_kwds = dockerfiles.docker_host_args(**kwds) + + def create_database(self, identifier): + # Not needed for profile creation, database will be created when Galaxy starts. + pass + + def delete_database(self, identifier): + # Not needed for profile deletion, just remove the profile directory. + pass + + def start(self): if not is_running_container(**self._docker_host_kwds): start_postgres_docker(**self._docker_host_kwds) # Hack to give docker a bit of time to boot up and allow psql to start. time.sleep(30) + def stop(self): + if is_running_container(**self._docker_host_kwds): + stop_postgres_docker(**self._docker_host_kwds) + def sqlalchemy_url(self, identifier): """Return URL or form postgresql://username:password@localhost/mydatabase.""" return "postgresql://%s:%s@%s:%d/%s" % ( diff --git a/planemo/database/postgres_singularity.py b/planemo/database/postgres_singularity.py index e0e21b8fb..b572f82f6 100644 --- a/planemo/database/postgres_singularity.py +++ b/planemo/database/postgres_singularity.py @@ -1,11 +1,12 @@ """Module describes a :class:`DatabaseSource` for managed, dockerized postgres databases.""" import os -import subprocess +import shutil import time from tempfile import mkdtemp +from typing import Optional -from galaxy.util.commands import execute +from galaxy.util.commands import shell_process from planemo.io import info from .interface import DatabaseSource @@ -15,21 +16,15 @@ DEFAULT_POSTGRES_DATABASE_NAME = "galaxy" DEFAULT_POSTGRES_USER = "galaxy" DEFAULT_POSTGRES_PASSWORD = "mysecretpassword" -DEFAULT_POSTGRES_PORT_EXPOSE = 5432 DEFAULT_DOCKERIMAGE = "postgres:14.2-alpine3.15" -DEFAULT_SIF_NAME = "postgres_14_2-alpine3_15.sif" - -DEFAULT_CONNECTION_STRING = f"postgresql://{DEFAULT_POSTGRES_USER}:{DEFAULT_POSTGRES_PASSWORD}@localhost:{DEFAULT_POSTGRES_PORT_EXPOSE}/{DEFAULT_POSTGRES_DATABASE_NAME}" def start_postgres_singularity( singularity_path, - container_instance_name, database_location, databasename=DEFAULT_POSTGRES_DATABASE_NAME, user=DEFAULT_POSTGRES_USER, password=DEFAULT_POSTGRES_PASSWORD, - **kwds, ): info(f"Postgres database stored at: {database_location}") pgdata_path = os.path.join(database_location, "pgdata") @@ -40,66 +35,41 @@ def start_postgres_singularity( if not os.path.exists(pgrun_path): os.makedirs(pgrun_path) - version_file = os.path.join(pgdata_path, "PG_VERSION") - if not os.path.exists(version_file): - # Run container for a short while to initialize the database - # The database will not be initilizaed during a - # "singularity instance start" command - init_database_command = [ - singularity_path, - "run", - "-B", - f"{pgdata_path}:/var/lib/postgresql/data", - "-B", - f"{pgrun_path}:/var/run/postgresql", - "-e", - "-C", - "--env", - f"POSTGRES_DB={databasename}", - "--env", - f"POSTGRES_USER={user}", - "--env", - f"POSTGRES_PASSWORD={password}", - "--env", - "POSTGRES_INITDB_ARGS='--encoding=UTF-8'", - f"docker://{DEFAULT_DOCKERIMAGE}", - ] - info(f"Initilizing postgres database in folder: {pgdata_path}") - process = subprocess.Popen(init_database_command, stdout=subprocess.PIPE, stderr=subprocess.PIPE) - # Give the container time to initialize the database - for _ in range(10): - if os.path.exists(version_file): - break - time.sleep(5) - info("Waiting for the postgres database to initialize.") - else: - raise Exception("Failed to initialize the postgres database.") - time.sleep(10) - process.terminate() - - # Start the singularity instance, assumes the database is - # already initialized since the entrypoint will not be run - # when starting a instance of the container. + VERSION_FILE = os.path.join(pgdata_path, "PG_VERSION") run_command = [ singularity_path, - "instance", - "start", + "run", "-B", f"{pgdata_path}:/var/lib/postgresql/data", "-B", f"{pgrun_path}:/var/run/postgresql", "-e", "-C", + "--env", + f"POSTGRES_DB={databasename}", + "--env", + f"POSTGRES_USER={user}", + "--env", + f"POSTGRES_PASSWORD={password}", + "--env", + "POSTGRES_INITDB_ARGS='--encoding=UTF-8'", f"docker://{DEFAULT_DOCKERIMAGE}", - container_instance_name, ] - info(f"Starting singularity instance named: {container_instance_name}") - execute(run_command) - - -def stop_postgress_singularity(container_instance_name, **kwds): - info(f"Stopping singularity instance named: {container_instance_name}") - execute(["singularity", "instance", "stop", container_instance_name]) + info("Starting postgres singularity container") + p = shell_process(run_command) + # Give the container time to initialize the database + for _ in range(10): + if os.path.exists(VERSION_FILE): + break + time.sleep(5) + info("Waiting for the postgres database to initialize.") + else: + try: + p.terminate() + except Exception as e: + info(f"Failed to terminate process: {e}") + raise Exception("Failed to initialize the postgres database.") + return p class SingularityPostgresDatabaseSource(ExecutesPostgresSqlMixin, DatabaseSource): @@ -108,42 +78,58 @@ class SingularityPostgresDatabaseSource(ExecutesPostgresSqlMixin, DatabaseSource "with" statements to automatically start and stop the container. """ - def __init__(self, **kwds): + def __init__(self, profile_directory: Optional[str] = None, **kwds): """Construct a postgres database source from planemo configuration.""" self.singularity_path = "singularity" self.database_user = DEFAULT_POSTGRES_USER self.database_password = DEFAULT_POSTGRES_PASSWORD - self.database_host = "localhost" # TODO: Make docker host - self.database_port = DEFAULT_POSTGRES_PORT_EXPOSE - if "postgres_storage_location" in kwds and kwds["postgres_storage_location"] is not None: + if kwds.get("postgres_storage_location") is not None: self.database_location = kwds["postgres_storage_location"] + elif profile_directory: + self.database_location = os.path.join(profile_directory, "postgres") else: self.database_location = os.path.join(mkdtemp(suffix="_planemo_postgres_db")) + self.database_socket_dir = os.path.join(self.database_location, "pgrun") self.container_instance_name = f"{DEFAULT_CONTAINER_NAME}-{int(time.time() * 1000000)}" self._kwds = kwds + self.running_process = None + + def create_database(self, identifier): + # Not needed, we'll create the database automatically when the container starts. + pass + + def delete_database(self, identifier): + shutil.rmtree(self.database_location, ignore_errors=True) - def __enter__(self): - start_postgres_singularity( + def start(self): + self.running_process = start_postgres_singularity( singularity_path=self.singularity_path, database_location=self.database_location, user=self.database_user, password=self.database_password, - container_instance_name=self.container_instance_name, - **self._kwds, ) - def __exit__(self, exc_type, exc_value, traceback): - stop_postgress_singularity(self.container_instance_name) + def stop(self): + if self.running_process: + try: + self.running_process.terminate() + postmaster_pid_file = os.path.join(self.database_location, "pgdata", "postmaster.pid") + if os.path.exists(postmaster_pid_file): + os.remove(postmaster_pid_file) + postmaster_lock_file = os.path.join(self.database_location, "pgrun", ".s.PGSQL.5432.lock") + if os.path.exists(postmaster_lock_file): + os.remove(postmaster_lock_file) + except Exception as e: + info(f"Failed to terminate process: {e}") def sqlalchemy_url(self, identifier): - """Return URL or form postgresql://username:password@localhost/mydatabase.""" - return "postgresql://%s:%s@%s:%d/%s" % ( + """Return URL for PostgreSQL connection via Unix socket.""" + return "postgresql://%s:%s@/%s?host=%s" % ( self.database_user, self.database_password, - self.database_host, - self.database_port, identifier, + self.database_socket_dir, ) diff --git a/planemo/engine/factory.py b/planemo/engine/factory.py index 7d57b7d3b..6e81b036a 100644 --- a/planemo/engine/factory.py +++ b/planemo/engine/factory.py @@ -9,7 +9,6 @@ DockerizedManagedGalaxyEngine, ExternalGalaxyEngine, LocalManagedGalaxyEngine, - LocalManagedGalaxyEngineWithSingularityDB, ) from .toil import ToilEngine @@ -26,10 +25,7 @@ def build_engine(ctx, **kwds): """Build an engine from the supplied planemo configuration.""" engine_type_str = kwds.get("engine", "galaxy") if engine_type_str == "galaxy": - if "database_type" in kwds and kwds["database_type"] == "postgres_singularity": - engine_type = LocalManagedGalaxyEngineWithSingularityDB - else: - engine_type = LocalManagedGalaxyEngine + engine_type = LocalManagedGalaxyEngine elif engine_type_str == "docker_galaxy": engine_type = DockerizedManagedGalaxyEngine elif engine_type_str == "external_galaxy": diff --git a/planemo/engine/galaxy.py b/planemo/engine/galaxy.py index c56862dab..8b611f2f0 100644 --- a/planemo/engine/galaxy.py +++ b/planemo/engine/galaxy.py @@ -12,7 +12,6 @@ from galaxy.tool_util.verify import interactor from planemo import io -from planemo.database.postgres_singularity import SingularityPostgresDatabaseSource from planemo.galaxy.activity import ( execute, execute_rerun, @@ -172,13 +171,6 @@ def _serve_kwds(self): return self._kwds.copy() -class LocalManagedGalaxyEngineWithSingularityDB(LocalManagedGalaxyEngine): - def run(self, runnables, job_paths, output_collectors: Optional[List[Callable]] = None): - with SingularityPostgresDatabaseSource(**self._kwds.copy()): - run_responses = super().run(runnables, job_paths, output_collectors) - return run_responses - - class DockerizedManagedGalaxyEngine(LocalManagedGalaxyEngine): """An :class:`Engine` implementation backed by Galaxy running in Docker. @@ -233,5 +225,4 @@ def expand_test_cases(config, test_cases): "DockerizedManagedGalaxyEngine", "ExternalGalaxyEngine", "LocalManagedGalaxyEngine", - "LocalManagedGalaxyEngineWithSingularityDB", ) diff --git a/planemo/galaxy/config.py b/planemo/galaxy/config.py index d0e85f981..9c1693e28 100644 --- a/planemo/galaxy/config.py +++ b/planemo/galaxy/config.py @@ -37,7 +37,7 @@ from planemo import git from planemo.config import OptionSource -from planemo.database import postgres_singularity +from planemo.database import create_database_source from planemo.deps import ensure_dependency_resolvers_conf_configured from planemo.docker import docker_host_args from planemo.galaxy.workflows import ( @@ -422,7 +422,6 @@ def config_join(*args): ) ) _handle_container_resolution(ctx, kwds, properties) - properties["database_connection"] = _database_connection(database_location, **kwds) if kwds.get("mulled_containers", False): properties["mulled_channels"] = kwds.get("conda_ensure_channels", "") @@ -445,15 +444,6 @@ def config_join(*args): # https://github.com/galaxyproject/planemo/issues/788 env["GALAXY_LOG"] = log_file env["GALAXY_PID"] = pid_file - write_galaxy_config( - galaxy_root=galaxy_root, - properties=properties, - env=env, - kwds=kwds, - template_args=template_args, - config_join=config_join, - ) - _write_tool_conf(ctx, all_tool_paths, tool_conf) write_file(empty_tool_conf, EMPTY_TOOL_CONF_TEMPLATE) @@ -462,19 +452,29 @@ def config_join(*args): write_file(shed_tool_conf, shed_tool_conf_contents, force=False) write_file(shed_data_manager_config_file, SHED_DATA_MANAGER_CONF_TEMPLATE) + with _database_connection(database_location, **kwds) as database_connection: + properties["database_connection"] = database_connection + write_galaxy_config( + galaxy_root=galaxy_root, + properties=properties, + env=env, + kwds=kwds, + template_args=template_args, + config_join=config_join, + ) - yield LocalGalaxyConfig( - ctx, - config_directory, - env, - test_data_dir, - port, - server_name, - master_api_key, - runnables, - galaxy_root, - kwds, - ) + yield LocalGalaxyConfig( + ctx, + config_directory, + env, + test_data_dir, + port, + server_name, + master_api_key, + runnables, + galaxy_root, + kwds, + ) def write_galaxy_config(galaxy_root, properties, env, kwds, template_args, config_join): @@ -1105,13 +1105,17 @@ def default_use_path_paste(self): return self.user_is_admin +@contextlib.contextmanager def _database_connection(database_location, **kwds): - if "database_type" in kwds and kwds["database_type"] == "postgres_singularity": - default_connection = postgres_singularity.DEFAULT_CONNECTION_STRING + if kwds.get("database_type") != "sqlite": + database_source = create_database_source(**kwds) + try: + database_source.start() + yield database_source.sqlalchemy_url(kwds.get("database_identifier", "galaxy")) + finally: + database_source.stop() else: - default_connection = DATABASE_LOCATION_TEMPLATE % database_location - database_connection = kwds.get("database_connection") or default_connection - return database_connection + yield DATABASE_LOCATION_TEMPLATE % database_location def _find_galaxy_root(ctx, **kwds): diff --git a/planemo/galaxy/profiles.py b/planemo/galaxy/profiles.py index 99fd942f3..26f1b5c9f 100644 --- a/planemo/galaxy/profiles.py +++ b/planemo/galaxy/profiles.py @@ -96,7 +96,7 @@ def _create_profile_local(ctx, profile_directory, profile_name, kwds): else: database_type = "sqlite" - if database_type not in ["sqlite", "postgres_singularity"]: + if database_type not in ["sqlite"]: database_source = create_database_source(**kwds) database_identifier = _profile_to_database_identifier(profile_name) try: @@ -115,8 +115,10 @@ def _create_profile_local(ctx, profile_directory, profile_name, kwds): elif database_type == "postgres_singularity": database_connection + database_source.sqlalchemy_url(database_identifier) if database_type == "sqlite": - database_location = os.path.join(profile_directory, "galaxy.sqlite") - database_connection = DATABASE_LOCATION_TEMPLATE % database_location + database_source.create_database( + database_identifier, + ) + database_connection = database_source.sqlalchemy_url(database_identifier) return { "database_type": database_type, diff --git a/planemo/options.py b/planemo/options.py index 789fb39d9..01fc14a20 100644 --- a/planemo/options.py +++ b/planemo/options.py @@ -1723,7 +1723,7 @@ def postgres_datatype_type_option(): def postgres_database_storage_location_option(): return planemo_option( - "--postgres-storage-location", + "--postgres_storage_location", type=str, help="storage path for postgres database, used for local singularity postgres.", default=None, diff --git a/tests/test_database_commands.py b/tests/test_database_commands.py index 74494df17..e31a2573f 100644 --- a/tests/test_database_commands.py +++ b/tests/test_database_commands.py @@ -1,4 +1,3 @@ -from planemo.database.postgres_docker import stop_postgres_docker from .test_utils import ( CliTestCase, skip_unless_environ, @@ -27,7 +26,4 @@ def test_database_commands(self): @skip_unless_environ("PLANEMO_ENABLE_POSTGRES_TESTS") @skip_unless_executable("docker") def test_database_commands_docker(self): - try: - self._database_commands(database_type="postgres_docker") - finally: - stop_postgres_docker() + self._database_commands(database_type="postgres_docker") diff --git a/tests/test_profile_commands.py b/tests/test_profile_commands.py index bfac36903..9debb4640 100644 --- a/tests/test_profile_commands.py +++ b/tests/test_profile_commands.py @@ -1,4 +1,3 @@ -from planemo.database.postgres_docker import stop_postgres_docker from .test_utils import ( CliTestCase, skip_unless_environ, @@ -25,7 +24,4 @@ def test_profile_commands(self): @skip_unless_executable("docker") def test_profile_commands_docker(self): - try: - self._profile_commands(database_type="postgres_docker") - finally: - stop_postgres_docker() + self._profile_commands(database_type="postgres_docker")