diff --git a/.dockerignore b/.dockerignore index d9df5730..0521228c 100644 --- a/.dockerignore +++ b/.dockerignore @@ -3,7 +3,9 @@ # what the image needs; everything else (frontend, docs, tests, venvs) stays out. * !docsgpt/ -!application/ +# Only the alias file: an upgraded checkout may still hold gitignored +# application/{inputs,indexes,vectors,.env} from the old layout. +!application/__init__.py # Inside the package: caches, local runtime data and secrets never ship. **/__pycache__/ diff --git a/.github/workflows/pytest.yml b/.github/workflows/pytest.yml index 8b563bdb..3e31db75 100644 --- a/.github/workflows/pytest.yml +++ b/.github/workflows/pytest.yml @@ -20,7 +20,7 @@ jobs: - name: Install dependencies run: | python -m pip install --upgrade pip - cd application + cd docsgpt if [ -f requirements.txt ]; then pip install -r requirements.txt; fi cd ../tests if [ -f requirements.txt ]; then pip install -r requirements.txt; fi diff --git a/application/__init__.py b/application/__init__.py index 1863e1c8..49ce5dc8 100644 --- a/application/__init__.py +++ b/application/__init__.py @@ -3,8 +3,9 @@ Every ``import application.x.y`` resolves to the already-imported ``docsgpt.x.y`` module object, so there is exactly one settings object, one Celery app and one Flask app however a process refers to them. Entry points such as -``celery -A application.app.celery`` and ``uvicorn application.asgi:asgi_app`` -keep working; update them to ``docsgpt.…`` before the alias is removed. +``celery -A application.app.celery``, ``uvicorn application.asgi:asgi_app`` and +``python -m application.scripts.`` keep working; update them to +``docsgpt.…`` before the alias is removed. """ from __future__ import annotations @@ -20,16 +21,37 @@ _NEW = "docsgpt" class _AliasLoader(importlib.abc.Loader): - """Hand back the ``docsgpt`` module instead of executing anything.""" + """Hand back the ``docsgpt`` module object; delegate code access to its real loader.""" - def __init__(self, target: str) -> None: + def __init__(self, target: str, target_spec) -> None: self._target = target + self._target_spec = target_spec + self._original_spec = None def create_module(self, spec): - return importlib.import_module(self._target) + module = importlib.import_module(self._target) + self._original_spec = module.__spec__ + return module def exec_module(self, module) -> None: - return None + # The import machinery stamps the alias spec on the shared module + # object; put the real one back so importlib.reload and __spec__-based + # lookups keep addressing the module by its docsgpt name. + if self._original_spec is not None: + module.__spec__ = self._original_spec + + # runpy (``python -m application.x``) reads the code through the loader. + def get_code(self, fullname): + return self._target_spec.loader.get_code(self._target) + + def get_source(self, fullname): + return self._target_spec.loader.get_source(self._target) + + def get_filename(self, fullname): + return self._target_spec.loader.get_filename(self._target) + + def is_package(self, fullname): + return self._target_spec.submodule_search_locations is not None class _AliasFinder(importlib.abc.MetaPathFinder): @@ -39,12 +61,10 @@ class _AliasFinder(importlib.abc.MetaPathFinder): if name != _OLD and not name.startswith(_OLD + "."): return None new_name = _NEW + name[len(_OLD):] - spec = importlib.util.find_spec(new_name) - if spec is None: + target_spec = importlib.util.find_spec(new_name) + if target_spec is None: return None - return importlib.util.spec_from_loader( - name, _AliasLoader(new_name), is_package=spec.submodule_search_locations is not None - ) + return importlib.util.spec_from_loader(name, _AliasLoader(new_name, target_spec)) warnings.warn( diff --git a/deployment/docker-compose-azure.yaml b/deployment/docker-compose-azure.yaml index 10badc24..97b19295 100644 --- a/deployment/docker-compose-azure.yaml +++ b/deployment/docker-compose-azure.yaml @@ -44,9 +44,12 @@ services: ports: - "7091:7091" volumes: - - ../docsgpt/indexes:/app/docsgpt/indexes - - ../docsgpt/inputs:/app/docsgpt/inputs - - ../docsgpt/vectors:/app/docsgpt/vectors + # Host data stays under application/ (the old package directory) for this + # release so an upgraded checkout keeps its indexes; it moves with the + # packaging work, together with an upgrade note. + - ../application/indexes:/app/docsgpt/indexes + - ../application/inputs:/app/docsgpt/inputs + - ../application/vectors:/app/docsgpt/vectors depends_on: redis: condition: service_started diff --git a/deployment/docker-compose-hub.yaml b/deployment/docker-compose-hub.yaml index f31e5b1a..0f2a8a80 100644 --- a/deployment/docker-compose-hub.yaml +++ b/deployment/docker-compose-hub.yaml @@ -44,9 +44,12 @@ services: ports: - "7091:7091" volumes: - - ../docsgpt/indexes:/app/indexes - - ../docsgpt/inputs:/app/inputs - - ../docsgpt/vectors:/app/vectors + # Host data stays under application/ (the old package directory) for this + # release so an upgraded checkout keeps its indexes; it moves with the + # packaging work, together with an upgrade note. + - ../application/indexes:/app/indexes + - ../application/inputs:/app/inputs + - ../application/vectors:/app/vectors depends_on: redis: condition: service_started @@ -68,9 +71,12 @@ services: - CACHE_REDIS_URL=redis://redis:6379/2 - POSTGRES_URI=postgresql://docsgpt:docsgpt@postgres:5432/docsgpt volumes: - - ../docsgpt/indexes:/app/indexes - - ../docsgpt/inputs:/app/inputs - - ../docsgpt/vectors:/app/vectors + # Host data stays under application/ (the old package directory) for this + # release so an upgraded checkout keeps its indexes; it moves with the + # packaging work, together with an upgrade note. + - ../application/indexes:/app/indexes + - ../application/inputs:/app/inputs + - ../application/vectors:/app/vectors depends_on: redis: condition: service_started diff --git a/deployment/docker-compose.yaml b/deployment/docker-compose.yaml index 3fd4b6d6..0eadaece 100644 --- a/deployment/docker-compose.yaml +++ b/deployment/docker-compose.yaml @@ -58,9 +58,12 @@ services: ports: - "7091:7091" volumes: - - ../docsgpt/indexes:/app/indexes - - ../docsgpt/inputs:/app/inputs - - ../docsgpt/vectors:/app/vectors + # Host data stays under application/ (the old package directory) for this + # release so an upgraded checkout keeps its indexes; it moves with the + # packaging work, together with an upgrade note. + - ../application/indexes:/app/indexes + - ../application/inputs:/app/inputs + - ../application/vectors:/app/vectors depends_on: redis: condition: service_started @@ -93,9 +96,12 @@ services: - CACHE_REDIS_URL=redis://redis:6379/2 - POSTGRES_URI=postgresql://docsgpt:docsgpt@postgres:5432/docsgpt volumes: - - ../docsgpt/indexes:/app/indexes - - ../docsgpt/inputs:/app/inputs - - ../docsgpt/vectors:/app/vectors + # Host data stays under application/ (the old package directory) for this + # release so an upgraded checkout keeps its indexes; it moves with the + # packaging work, together with an upgrade note. + - ../application/indexes:/app/indexes + - ../application/inputs:/app/inputs + - ../application/vectors:/app/vectors depends_on: redis: condition: service_started diff --git a/docs/content/Deploying/Docker-Deploying.mdx b/docs/content/Deploying/Docker-Deploying.mdx index dfa75e99..00932c35 100644 --- a/docs/content/Deploying/Docker-Deploying.mdx +++ b/docs/content/Deploying/Docker-Deploying.mdx @@ -69,8 +69,8 @@ checkout. ## Using the Source Checkout With a clone of the repository, `deployment/docker-compose-hub.yaml` runs the -same pre-built images while keeping your data in `docsgpt/indexes`, -`docsgpt/inputs` and `docsgpt/vectors`, and `deployment/docker-compose.yaml` +same pre-built images while keeping your data in `application/indexes`, +`application/inputs` and `application/vectors`, and `deployment/docker-compose.yaml` builds the images from your working tree (for local changes, or a build with extra packages: `EXTRAS=docling` in `.env`). diff --git a/docs/content/upgrading.mdx b/docs/content/upgrading.mdx index a122dfcf..4e634525 100644 --- a/docs/content/upgrading.mdx +++ b/docs/content/upgrading.mdx @@ -90,6 +90,26 @@ need attention: Staying on `all-mpnet-base-v2` is a supported choice — it remains in the model registry and in `setup.sh`. You only need this section if you want to move to granite. +## Backend package renamed to `docsgpt` + +The backend's Python package is `docsgpt` (it was `application`), the name it +will carry on PyPI. For one release the old name keeps working through an +alias, so nothing breaks on upgrade, but update these before the alias goes: + +- Entry points: `celery -A docsgpt.app.celery worker`, + `uvicorn docsgpt.asgi:asgi_app`, `python -m docsgpt.scripts.`. The + `application.…` spellings still run and print a `FutureWarning`. The + compose files, Kubernetes manifests and setup scripts in the repository are + already updated; only custom copies need editing. +- Local image builds: the build context is the repository root, so use + `docker build -f docsgpt/Dockerfile .` (or the compose files, which do this). +- Celery task names changed with the package (`docsgpt.api.user.tasks.ingest` + and so on). A worker on this release also accepts the old names, so tasks + queued before the upgrade still run, and beat rewrites the periodic + schedule in Redis on start-up. Nothing to do. +- Data directories do not move: the compose files keep your indexes, inputs + and vectors under `application/` in the checkout, where they already are. + ## Check your version ```bash diff --git a/docsgpt/Dockerfile b/docsgpt/Dockerfile index ace3c1c2..02c36636 100644 --- a/docsgpt/Dockerfile +++ b/docsgpt/Dockerfile @@ -138,7 +138,7 @@ RUN if python -c "import docling" 2>/dev/null; then \ COPY --chown=appuser:appuser docsgpt /app/docsgpt # One-release alias so `-A application.app.celery` style entry points keep working. -COPY --chown=appuser:appuser application /app/application +COPY --chown=appuser:appuser application/__init__.py /app/application/__init__.py # Runtime data directories, owned by the process user so a named volume # mounted on them (docker-compose-standalone.yaml) inherits that ownership diff --git a/docsgpt/api/user/tasks.py b/docsgpt/api/user/tasks.py index 8e822f7b..d4307b33 100644 --- a/docsgpt/api/user/tasks.py +++ b/docsgpt/api/user/tasks.py @@ -558,14 +558,17 @@ def setup_periodic_tasks(sender, **kwargs): sender.add_periodic_task( timedelta(days=1), schedule_syncs.s("daily"), + name="schedule-syncs-daily", ) sender.add_periodic_task( timedelta(weeks=1), schedule_syncs.s("weekly"), + name="schedule-syncs-weekly", ) sender.add_periodic_task( timedelta(days=30), schedule_syncs.s("monthly"), + name="schedule-syncs-monthly", ) # Replaces Mongo's TTL index on pending_tool_state.expires_at. sender.add_periodic_task( diff --git a/docsgpt/celery_init.py b/docsgpt/celery_init.py index eacd101e..02da0711 100644 --- a/docsgpt/celery_init.py +++ b/docsgpt/celery_init.py @@ -176,19 +176,27 @@ def register_legacy_task_names(app: Celery) -> int: """Make every ``docsgpt.*`` task answer to its old ``application.*`` name too. Messages queued by the previous release carry the old names; without the - alias a worker on this release rejects them as unregistered. Kept for one - release, together with the ``application`` import alias. + alias a worker on this release rejects them as unregistered. Each alias is + a distinct task object (a subclass carrying the old name), not the same + object under a second key: Celery builds its execution tracer per task + object, and one object under two names would log every run under + whichever name was traced last. Kept for one release, together with the + ``application`` import alias. Returns: The number of aliases added. """ added = 0 for name, task in list(app.tasks.items()): - if name.startswith("docsgpt."): - legacy = LEGACY_TASK_PREFIX + name[len("docsgpt."):] - if legacy not in app.tasks: - app.tasks[legacy] = task - added += 1 + if not name.startswith("docsgpt."): + continue + legacy = LEGACY_TASK_PREFIX + name[len("docsgpt."):] + if legacy in app.tasks: + continue + base = type(task) + legacy_cls = type(base.__name__, (base,), {"name": legacy, "__module__": base.__module__, "__doc__": base.__doc__}) + app.register_task(legacy_cls()) + added += 1 return added diff --git a/docsgpt/celeryconfig.py b/docsgpt/celeryconfig.py index 25e702aa..ee50bc08 100644 --- a/docsgpt/celeryconfig.py +++ b/docsgpt/celeryconfig.py @@ -50,10 +50,7 @@ task_queues = tuple( beat_scheduler = "redbeat.RedBeatScheduler" redbeat_redis_url = broker_url -# v2: the task names changed with the package rename; a new prefix leaves the -# schedule entries the previous release wrote in Redis unread instead of firing -# the old names alongside the new ones. -redbeat_key_prefix = "redbeat:docsgpt:v2:" +redbeat_key_prefix = "redbeat:docsgpt:" redbeat_lock_timeout = 90 # Survive worker SIGKILL/OOM without silently dropping in-flight tasks. diff --git a/tests/api/user/test_tasks.py b/tests/api/user/test_tasks.py index afc4a18f..a593bbb5 100644 --- a/tests/api/user/test_tasks.py +++ b/tests/api/user/test_tasks.py @@ -278,6 +278,19 @@ class TestIngestConnectorTask: class TestSetupPeriodicTasks: + @pytest.mark.unit + def test_every_entry_has_a_stable_name(self): + """Unnamed entries get keyed by the task path, which redbeat cannot update in place when it changes.""" + from docsgpt.api.user.tasks import setup_periodic_tasks + + sender = MagicMock() + setup_periodic_tasks(sender) + + names = [call.kwargs.get("name") for call in sender.add_periodic_task.call_args_list] + assert all(names), names + assert len(set(names)) == len(names), names + assert names[:3] == ["schedule-syncs-daily", "schedule-syncs-weekly", "schedule-syncs-monthly"] + @pytest.mark.unit def test_registers_periodic_tasks(self): from docsgpt.api.user.tasks import setup_periodic_tasks diff --git a/tests/test_package_rename.py b/tests/test_package_rename.py index 1b02707a..b448a987 100644 --- a/tests/test_package_rename.py +++ b/tests/test_package_rename.py @@ -1,30 +1,59 @@ """The ``application`` alias and the legacy Celery task names survive the rename to ``docsgpt``.""" import importlib +import runpy import sys import warnings import pytest +def _quiet_import(name): + with warnings.catch_warnings(): + warnings.simplefilter("ignore", FutureWarning) + return importlib.import_module(name) + + class TestApplicationAlias: def test_old_import_is_the_same_module_object(self): - with warnings.catch_warnings(): - warnings.simplefilter("ignore", FutureWarning) - old = importlib.import_module("application.vectorstore.model_registry") + old = _quiet_import("application.vectorstore.model_registry") new = importlib.import_module("docsgpt.vectorstore.model_registry") assert old is new assert sys.modules["application.vectorstore.model_registry"] is new def test_old_package_is_the_new_package(self): - with warnings.catch_warnings(): - warnings.simplefilter("ignore", FutureWarning) - import application # noqa: F401 + _quiet_import("application") assert sys.modules["application"] is importlib.import_module("docsgpt") + def test_alias_keeps_the_real_spec(self): + """Importing through the alias must not rename the shared module object.""" + module = _quiet_import("application.vectorstore.model_registry") + assert module.__spec__.name == "docsgpt.vectorstore.model_registry" + assert module.__name__ == "docsgpt.vectorstore.model_registry" + + def test_reload_works_on_the_shared_module(self): + # docsgpt.version has no shared constants, so re-executing it in place is harmless. + _quiet_import("application.version") + module = importlib.import_module("docsgpt.version") + reloaded = importlib.reload(module) + assert reloaded is module + assert reloaded.__spec__.name == "docsgpt.version" + assert reloaded.get_version() == module.__version__ + + def test_python_dash_m_through_the_alias(self): + """``python -m application.x`` goes through runpy, which needs the loader's get_code. + + A real ``python -m`` starts a fresh interpreter, so the target module is + not yet in ``sys.modules`` under the alias name when runpy looks it up; + drop any entry an earlier test left so the lookup reaches the finder. + """ + _quiet_import("application") + sys.modules.pop("application.vectorstore.model_registry", None) + globals_ = runpy.run_module("application.vectorstore.model_registry", run_name="__main__", alter_sys=True) + assert "DEFAULT_NEW_INSTALL" in globals_ + def test_alias_warns_when_first_imported(self): """The shim warns once per process: on the import that executes it.""" - # Undo a previous import so the shim module body runs again. sys.meta_path[:] = [f for f in sys.meta_path if type(f).__name__ != "_AliasFinder"] for name in [m for m in sys.modules if m == "application" or m.startswith("application.")]: del sys.modules[name] @@ -33,26 +62,28 @@ class TestApplicationAlias: assert sys.modules["application"] is importlib.import_module("docsgpt") def test_missing_module_still_raises(self): - with warnings.catch_warnings(): - warnings.simplefilter("ignore", FutureWarning) - with pytest.raises(ModuleNotFoundError): - importlib.import_module("application.no_such_module") + _quiet_import("application") + with pytest.raises(ModuleNotFoundError): + importlib.import_module("application.no_such_module") class TestLegacyTaskNames: - def test_every_task_answers_to_its_old_name(self): + def test_every_task_answers_to_its_old_name_as_its_own_object(self): from docsgpt.celery_init import LEGACY_TASK_PREFIX, celery, register_legacy_task_names importlib.import_module("docsgpt.api.user.tasks") importlib.import_module("docsgpt.vectorstore.embeddings_tasks") - added = register_legacy_task_names(celery) + register_legacy_task_names(celery) new_names = [n for n in celery.tasks if n.startswith("docsgpt.")] assert new_names, "no docsgpt.* tasks registered" for name in new_names: - legacy = LEGACY_TASK_PREFIX + name[len("docsgpt."):] - assert celery.tasks[legacy] is celery.tasks[name] + legacy_name = LEGACY_TASK_PREFIX + name[len("docsgpt."):] + canonical, legacy = celery.tasks[name], celery.tasks[legacy_name] + assert legacy is not canonical, "an alias must be its own task object (its own tracer)" + assert isinstance(legacy, type(canonical)) + assert legacy.name == legacy_name + assert canonical.name == name assert register_legacy_task_names(celery) == 0, "second call must be idempotent" - assert added <= len(new_names) def test_routes_and_reclaim_list_use_new_names(self): from docsgpt import celeryconfig @@ -60,4 +91,4 @@ class TestLegacyTaskNames: assert all(k.startswith("docsgpt.") for k in celeryconfig.task_routes) assert all(k.startswith("docsgpt.") for k in _NO_RECLAIM_TASKS) - assert celeryconfig.redbeat_key_prefix == "redbeat:docsgpt:v2:" + assert celeryconfig.redbeat_key_prefix == "redbeat:docsgpt:", "the prefix must stay: redbeat updates named entries in place"