From 869e56eaf0d76da8815d42c13d878d3fecab28dd Mon Sep 17 00:00:00 2001 From: Timothy Allen Date: Fri, 2 Oct 2026 21:37:26 +0200 Subject: [PATCH] Do not make the ingest folder A folder that Bindery made hid a missing volume mount. The books went to a folder inside the container, and CWA never saw them. A missing folder is now an error in the log at the start, and each upload fails with a message that names the mount. --- AGENTS.md | 1 + README.md | 7 ++++++- bindery/config.py | 12 +++++++++--- bindery/ingest.py | 13 +++++++++++-- bindery/orchestrator.py | 4 ++-- docker-compose.yml | 2 +- tests/conftest.py | 1 + tests/test_build_keys.py | 15 +++++++++++++++ tests/test_calibre_db.py | 1 + tests/test_config.py | 13 +++++++++++++ tests/test_cwa.py | 14 +++++++++++++- tests/test_deliver.py | 1 + tests/test_optional_cwa.py | 13 +++++++------ 13 files changed, 81 insertions(+), 16 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 80d5c05..90357bd 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -17,6 +17,7 @@ Keep the quotes around the glob in the node command. Without them, node reads `t Start a development server with no CWA: ```bash +mkdir -p dev/ingest CONFIG_DIR=dev/config INGEST_DIR=dev/ingest TMP_DIR=dev/tmp uv run python -m bindery ``` diff --git a/README.md b/README.md index 6b7bc97..3698014 100644 --- a/README.md +++ b/README.md @@ -69,7 +69,7 @@ Environment variables set the configuration. **Restart the container after a cha | `SESSION_COOKIE_SECURE` | `true` if users reach Bindery only over HTTPS | `false` | | `TRUST_PROXY` | `true` if a reverse proxy sends the client address. See "Lockouts and sessions". | `false` | | `URL_BASE` | A path prefix, such as `/bindery`. See "Reverse proxy". | None | -| `INGEST_DIR` | The CWA ingest folder. If unset, all uploads stay paused. | None | +| `INGEST_DIR` | The CWA ingest folder. If unset, all uploads stay paused. Bindery does not make this folder. | None | | `CONFIG_DIR` | The story database and `personal.ini` | `/config` | | `TMP_DIR` | The folder for the built ePubs. Bindery keeps the last build of each story. | `/tmp/bindery` | | `LOG_LEVEL` | The log level | `INFO` | @@ -263,6 +263,10 @@ The health check at `/api/health` works with and without the prefix. Bindery gives every book to CWA through the ingest folder. CWA makes all changes to the library. +**Bindery does not make the ingest folder.** If the folder in `INGEST_DIR` does not exist, most often +because its volume mount is missing, the log shows an error at the start and each upload fails. The +books go to CWA with the next update after you fix the mount. + 1. Bindery puts the ePub of a new story into the ingest folder. CWA imports it as a new book. 2. At the next update, Bindery finds the CWA book of the story. If you set `CALIBRE_DB_PATH`, it first looks for the story URL in the library database. Without a single match there, it searches @@ -530,6 +534,7 @@ Without CWA, Bindery puts each new build of a story into its ingest folder as an enough to work on downloads, chapters, rules and the web interface. ```bash +mkdir -p dev/ingest CONFIG_DIR=dev/config INGEST_DIR=dev/ingest TMP_DIR=dev/tmp uv run python -m bindery ``` diff --git a/bindery/config.py b/bindery/config.py index a4173b8..7ea774b 100644 --- a/bindery/config.py +++ b/bindery/config.py @@ -99,9 +99,15 @@ BACKUP_COUNT = 7 # daily backups to keep def prepare() -> None: """Make the folders, and give the user a personal.ini to edit on first start.""" - for directory in (CONFIG_DIR, UPLOAD_DIR, TMP_DIR, INGEST_DIR): - if directory is not None: - directory.mkdir(parents=True, exist_ok=True) + for directory in (CONFIG_DIR, UPLOAD_DIR, TMP_DIR): + directory.mkdir(parents=True, exist_ok=True) + # The ingest folder belongs to CWA. A folder that Bindery makes hides a missing volume mount, and CWA + # never sees its files. + if INGEST_DIR and not INGEST_DIR.is_dir(): + logger.error( + f"The ingest folder {INGEST_DIR} does not exist. Check INGEST_DIR and the volume mount. " + "Uploads fail until the folder exists." + ) if not PERSONAL_INI.exists(): shutil.copy(DEFAULT_PERSONAL_INI, PERSONAL_INI) logger.info(f"Created the default FanFicFare configuration at {PERSONAL_INI}") diff --git a/bindery/ingest.py b/bindery/ingest.py index 8c91eca..29d9de0 100644 --- a/bindery/ingest.py +++ b/bindery/ingest.py @@ -26,10 +26,19 @@ def sanitize_filename(name: str) -> str: return name[:MAX_NAME_LENGTH].strip(" .") or "story" +class IngestError(Exception): + """The ingest folder cannot take a book.""" + + class IngestFolder: def __init__(self, path: Path) -> None: self.path = Path(path) + def _check(self) -> None: + # Bindery does not make the folder. A missing folder is most often a missing volume mount. + if not self.path.is_dir(): + raise IngestError(f"The ingest folder {self.path} does not exist. Check INGEST_DIR and the volume mount.") + def _place(self, epub: Path, final_path: Path) -> Path: """Copy an epub in under a temporary .part name, then rename it. @@ -43,7 +52,7 @@ class IngestFolder: def ingest(self, epub: Path, name: str) -> Path: """Send a new book to CWA.""" - self.path.mkdir(parents=True, exist_ok=True) + self._check() # A short unique suffix, so a file that CWA has not imported yet is never replaced. stem = sanitize_filename(f"{name[:80]} {uuid.uuid4().hex[:6]}") final_path = self._place(epub, self.path / f"{stem}.epub") @@ -52,7 +61,7 @@ class IngestFolder: def send_update(self, epub: Path, book_id: int, name: str) -> Path: """Send a new epub for an existing book. CWA replaces the book's epub.""" - self.path.mkdir(parents=True, exist_ok=True) + self._check() # A unique name, so two updates of one book never share a sidecar file. stem = sanitize_filename(f"{name[:60]} {book_id}-{uuid.uuid4().hex[:8]}") final_path = self.path / f"{stem}.epub" diff --git a/bindery/orchestrator.py b/bindery/orchestrator.py index 12a3973..90781cb 100644 --- a/bindery/orchestrator.py +++ b/bindery/orchestrator.py @@ -17,7 +17,7 @@ from bindery.cwa import Book, BookFinder, CwaClient, CwaError from bindery.diffing import diff_html, text_hash from bindery.epub import EpubError, epub_metadata, epub_urls from bindery.fff import StoryError -from bindery.ingest import IngestFolder +from bindery.ingest import IngestError, IngestFolder from bindery.jobs import METADATA_FIELDS, DownloadMode, Job, QueueStatus, job_queue from bindery.merge import merge_chapter_lists from bindery.rules import RuleError, Step, apply_rules, compile_rules, normalize_rules @@ -1332,7 +1332,7 @@ def _run_next_job() -> Job | None: message = _run_job(job) job_queue.update(job, status=QueueStatus.DONE, message=message, progress=1.0) logger.info(f"Job {job.id} ({job.mode.value}) for {job.title}: {message}") - except (StoryError, CwaError, EpubError, RuleError, KeyError, ValueError) as e: + except (StoryError, CwaError, EpubError, IngestError, RuleError, KeyError, ValueError) as e: _fail(job, str(e)) except Exception as e: logger.exception(f"Job {job.id} failed: {e}") diff --git a/docker-compose.yml b/docker-compose.yml index 87b8447..dce788d 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -34,7 +34,7 @@ services: start_period: 30s retries: 3 volumes: - # CWA's ingest folder. New stories and updates go here. + # CWA's ingest folder. New stories and updates go here. Bindery does not make the folder. - /path/to/cwa/ingest:/cwa-book-ingest # The story database and personal.ini. Create the folder before the first start. - ./config:/config diff --git a/tests/conftest.py b/tests/conftest.py index 3a2cabc..80d60df 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -7,6 +7,7 @@ from pathlib import Path _tmp = Path(tempfile.mkdtemp(prefix="bindery-tests-")) for name, sub in [("CONFIG_DIR", "config"), ("INGEST_DIR", "ingest"), ("TMP_DIR", "tmp")]: os.environ[name] = str(_tmp / sub) +(_tmp / "ingest").mkdir() # Bindery does not make the ingest folder os.environ["BINDERY_START_THREADS"] = "false" os.environ["UPDATE_INTERVAL_HOURS"] = "0" for name in ( diff --git a/tests/test_build_keys.py b/tests/test_build_keys.py index 6a3987e..611076e 100644 --- a/tests/test_build_keys.py +++ b/tests/test_build_keys.py @@ -279,3 +279,18 @@ def test_the_start_deletes_uploads_that_no_job_needs(builds, jobs, uploads, stor orchestrator.queue_epub_file(waiting, "story.epub") orchestrator.remove_stale_uploads() assert list(uploads.iterdir()) == [waiting] + + +def test_a_missing_ingest_folder_fails_the_job_until_the_folder_exists(builds, jobs, monkeypatch, tmp_path): + monkeypatch.setattr(config, "INGEST_DIR", tmp_path / "not-mounted") + story_id = linked_story(ONE) + site(monkeypatch, ONE) + monkeypatch.setattr(orchestrator.time, "sleep", Mock()) + jobs.add(Job(mode=DownloadMode.NEW, story_id=story_id)) + job = orchestrator._run_next_job() + assert job.status == "error" and "does not exist. Check INGEST_DIR and the volume mount." in job.message + assert story(story_id)["status"] == "error" + assert not config.INGEST_DIR.exists() + config.INGEST_DIR.mkdir() + assert "library book 1" in update(story_id) + assert builds.call_count == 1 diff --git a/tests/test_calibre_db.py b/tests/test_calibre_db.py index 2abee32..d195a54 100644 --- a/tests/test_calibre_db.py +++ b/tests/test_calibre_db.py @@ -250,6 +250,7 @@ def test_library_database_failure_keeps_the_http_lookup(library_db, monkeypatch, def test_database_book_id_reaches_the_epub_sidecar(client, library_db, monkeypatch, tmp_path, saved_id, saved_uuid): monkeypatch.setattr(config, "CALIBRE_DB_PATH", library_db) monkeypatch.setattr(config, "INGEST_DIR", tmp_path / "ingest") + config.INGEST_DIR.mkdir(exist_ok=True) remote = CwaClient("") remote.session.get = Mock(side_effect=AssertionError("No HTTP lookup needed")) monkeypatch.setattr(orchestrator, "cwa", lambda: remote) diff --git a/tests/test_config.py b/tests/test_config.py index 68a2cf5..0b7b6d1 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -188,3 +188,16 @@ def test_prepare_warns_about_a_cwa_password_without_a_user_name(monkeypatch, tmp with caplog.at_level("WARNING"): config.prepare() assert ("CWA_PASSWORD is set" in caplog.text) is warns + + +@pytest.mark.parametrize("exists", [False, True]) +def test_prepare_does_not_make_the_ingest_folder(monkeypatch, tmp_path, caplog, exists): + monkeypatch.setattr(config, "CWA_URL", "https://cwa.invalid") + for name, folder in [("CONFIG_DIR", "config"), ("UPLOAD_DIR", "uploads"), ("TMP_DIR", "tmp"), ("INGEST_DIR", "in")]: + monkeypatch.setattr(config, name, tmp_path / folder) + monkeypatch.setattr(config, "PERSONAL_INI", tmp_path / "config" / "personal.ini") + if exists: + config.INGEST_DIR.mkdir() + config.prepare() + assert config.INGEST_DIR.exists() is exists + assert ("does not exist. Check INGEST_DIR and the volume mount" in caplog.text) is not exists diff --git a/tests/test_cwa.py b/tests/test_cwa.py index edd2493..c09ee05 100644 --- a/tests/test_cwa.py +++ b/tests/test_cwa.py @@ -9,7 +9,7 @@ from test_build import URL, chapters, metahtml from bindery import fff from bindery.cwa import BookFinder, CwaClient, CwaError, plain_title, search_text, title_rank from bindery.epub import epub_metadata, epub_urls -from bindery.ingest import MAX_NAME_LENGTH, IngestFolder +from bindery.ingest import MAX_NAME_LENGTH, IngestError, IngestFolder STORY_URL = "https://www.royalroad.com/fiction/1234" @@ -187,6 +187,7 @@ def test_epub_reader(story_epub, tmp_path): def test_ingest_uses_part_names_and_unique_names(tmp_path): folder = IngestFolder(tmp_path / "ingest") + folder.path.mkdir() epub = tmp_path / "x.epub" epub.write_bytes(b"PK") first = folder.ingest(epub, 'A: "Story" / Name') @@ -198,6 +199,7 @@ def test_ingest_uses_part_names_and_unique_names(tmp_path): def test_send_update_writes_sidecar(tmp_path): folder = IngestFolder(tmp_path / "ingest") + folder.path.mkdir() epub = tmp_path / "x.epub" epub.write_bytes(b"PK") path = folder.send_update(epub, 7, "T" * 300) @@ -206,3 +208,13 @@ def test_send_update_writes_sidecar(tmp_path): manifest = json.loads((folder.path / (path.name + ".cwa.json")).read_text()) assert manifest == {"action": "add_format", "book_id": 7} assert path.read_bytes() == b"PK" + + +def test_a_missing_ingest_folder_is_an_error_and_is_not_made(tmp_path, story_epub): + source = tmp_path / "book.epub" + source.write_bytes(story_epub) + folder = IngestFolder(tmp_path / "missing" / "ingest") + for send in (lambda: folder.ingest(source, "Title"), lambda: folder.send_update(source, 5, "Title")): + with pytest.raises(IngestError, match="does not exist. Check INGEST_DIR and the volume mount."): + send() + assert not (tmp_path / "missing").exists() diff --git a/tests/test_deliver.py b/tests/test_deliver.py index b08fa45..003d565 100644 --- a/tests/test_deliver.py +++ b/tests/test_deliver.py @@ -35,6 +35,7 @@ def fake(story_epub, monkeypatch, tmp_path): monkeypatch.setattr(config, "CWA_PASSWORD", "secret") monkeypatch.setattr(orchestrator, "store", Store(tmp_path / "bindery.db")) monkeypatch.setattr(config, "INGEST_DIR", tmp_path / "ingest") + config.INGEST_DIR.mkdir() monkeypatch.setattr(orchestrator, "finder", orchestrator.BookFinder()) yield server server.close() diff --git a/tests/test_optional_cwa.py b/tests/test_optional_cwa.py index a83e7b9..862f5ab 100644 --- a/tests/test_optional_cwa.py +++ b/tests/test_optional_cwa.py @@ -21,6 +21,7 @@ from bindery.store import Store @pytest.fixture(autouse=True) def isolated(client, jobs, monkeypatch, tmp_path): monkeypatch.setattr(config, "INGEST_DIR", tmp_path / "ingest") + config.INGEST_DIR.mkdir() monkeypatch.setattr(config, "TMP_DIR", tmp_path / "tmp") config.TMP_DIR.mkdir() monkeypatch.setattr(config, "CWA_URL", "") @@ -151,7 +152,7 @@ def test_build_without_ingest_succeeds_and_keeps_the_book_local(monkeypatch, tmp assert story["status"] == "ok" assert story["ingested_at"] == ("2026-01-01" if existing else None) assert list(config.TMP_DIR.glob(f"story-{story_id}-*.epub")) - assert not (tmp_path / "ingest").exists() + assert not list((tmp_path / "ingest").iterdir()) details = orchestrator.story_details(story_id) assert details["uploads_paused"] is True assert details["story"]["upload_held"] is True @@ -184,7 +185,7 @@ def test_linking_off_exports_books_and_respects_pause(monkeypatch, tmp_path, con assert orchestrator.store.get_setting(story_id, "upload_held") is None else: assert message.startswith("Not sent.") - assert not config.INGEST_DIR.exists() + assert not list(config.INGEST_DIR.iterdir()) assert orchestrator.store.get_setting(story_id, "upload_held") is True @@ -221,7 +222,7 @@ def test_enabling_linking_restores_the_saved_approval_preference(library): assert orchestrator.get_settings()["hold_new_links"] is True assert orchestrator.story_details(story_id)["story"]["needs_approval"] is True assert "Approve link" in rebuild(story_id) - assert not config.INGEST_DIR.exists() + assert not list(config.INGEST_DIR.iterdir()) def test_linking_off_exports_every_rebuild_without_an_id_or_approval(): @@ -294,7 +295,7 @@ def test_paused_unlinked_update_does_not_raise_not_found(monkeypatch): story_id = add_story(ingested_at="2026-01-01") assert rebuild(story_id) == "Not sent. Turn off pause to send." assert orchestrator.store.get_story(story_id)["status"] == "ok" - assert not config.INGEST_DIR.exists() + assert not list(config.INGEST_DIR.iterdir()) def test_enabling_linking_rebuilds_held_books_and_matches_again(monkeypatch, jobs, tmp_path): @@ -335,7 +336,7 @@ def test_upload_settings_matrix(client, link_books, hold_new_links, paused, save assert listed["upload_held"] is pending assert listed["approval_held"] is (link_books and hold_new_links and not paused) assert bool(orchestrator.store.get_setting(story_id, "upload_held")) is pending - assert config.INGEST_DIR.exists() is (not pending) + assert bool(list(config.INGEST_DIR.iterdir())) is (not pending) @pytest.mark.parametrize("missing_ingest", [False, True]) @@ -376,7 +377,7 @@ def test_resuming_uploads_does_not_bypass_approval(client, jobs, library): assert client.get("/api/stories").json[0]["approval_held"] is True assert "Approve link" in orchestrator._run_rebuild(jobs.get_next()) assert orchestrator.store.get_setting(story_id, "upload_held") is True - assert not config.INGEST_DIR.exists() + assert not list(config.INGEST_DIR.iterdir()) def test_missing_library_connection_is_not_an_approval_hold(client, monkeypatch):