mirror of
https://github.com/tiennm99/DocsGPT.git
synced 2026-10-11 12:11:45 +00:00
fix(rename): address review on the package rename
- CI installs the backend requirements from docsgpt/; the old cd into
application/ silently installed nothing.
- The root .dockerignore re-admits only application/__init__.py. An upgraded
checkout may still hold gitignored application/{inputs,indexes,vectors,.env}
from the old layout, and the directory rule shipped them into the image.
- The compose files keep the host bind mounts on application/{indexes,inputs,
vectors}, so an upgrade does not start with empty data. The move comes with
the packaging work, together with an upgrade note.
- The alias loader puts the real docsgpt spec back on the shared module object
after import (the import machinery stamped the alias spec on it, which made
importlib.reload rename the module and skip re-execution) and delegates
get_code/get_source/get_filename to the target loader, so
python -m application.<name> runs.
- Each legacy application.* task name is registered as its own task object,
a subclass carrying the old name. Registering the same object under two
keys made Celery's tracer log every run under whichever name it built last.
- The redbeat key prefix stays redbeat:docsgpt:; the three schedule_syncs
entries get stable names instead. redbeat tracks its static entries and
deletes the ones that vanish from beat_schedule at start-up, and rewrites the
task path of named entries in place, so neither a prefix bump nor a cleanup
pass is needed (checked against redbeat 2.4.2 with a seeded Redis).
This commit is contained in:
1 parent
574f96341e
commit
adb6963523
14 files changed
+168
-59
No files matched your search
+3
-1
@@ -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__/
|
||||
|
||||
@@ -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
|
||||
|
||||
+31
-11
@@ -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.<name>`` 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(
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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`).
|
||||
|
||||
|
||||
@@ -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.
|
||||
</Callout>
|
||||
|
||||
## 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.<name>`. 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
|
||||
|
||||
+1
-1
@@ -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
|
||||
|
||||
@@ -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(
|
||||
|
||||
+15
-7
@@ -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
|
||||
|
||||
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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"
|
||||
Reference in new issue
Block a user