Repository navigation
ci(unstructured2graph): download NLTK data explicitly - #258
Merged
Merged
Conversation
…silent auto-download Fixes #257. unstructured.nlp.tokenize auto-downloads averaged_perceptron_tagger_eng and punkt_tab on first import (AUTO_DOWNLOAD_NLTK, quiet=True by default) -- in the test-unstructured2graph CI job this wasn't succeeding, and since quiet=True suppresses nltk.download()'s own diagnostic output, the failure was silent and only surfaced later as 22 unrelated-looking pytest failures (LookupError deep inside unstructured's text-partitioning path), not as a clear "download failed" error. Splits the previous single "Install dependencies and run tests" step into three: Install dependencies, Download NLTK data (new -- explicit, visible `python -m nltk.downloader averaged_perceptron_tagger_eng punkt_tab`, no quiet flag), and Run tests. If the download ever fails again, this step fails loudly on its own instead of masquerading as broken test logic. Root cause of *why* the silent auto-download wasn't succeeding in CI specifically is still unconfirmed -- the download mechanism itself works fine given a real network path (verified locally against a fully isolated HOME, matching CI's fresh-runner conditions), so this doesn't appear to be a fundamental incompatibility with the nltk<3.10 pin from #251. This fix makes the dependency explicit and any future failure diagnosable, which was the actual problem (22 confusing test failures instead of one clear one), independent of pinning down the exact CI-side cause. Verified: full suite (74 passed, 1 skipped) against a live Memgraph + OpenAI in a from-scratch venv + isolated HOME, replicating the CI job's exact command sequence (uv venv -> uv pip install -e .[test] -> download -> pytest .).
The workflow's own paths filters (top-level trigger and each dorny/paths-filter entry) didn't include tests.yaml, so a PR that only edits this file never runs any test-* job -- including the NLTK-data fix in this PR, which touches only this workflow. Since every job is defined in this one shared file, an edit to it can affect any of them, so it's added everywhere memgraph-toolbox already is: the shared- dependency case.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #257.
unstructured.nlp.tokenizeauto-downloadsaveraged_perceptron_tagger_engandpunkt_tabon first import (AUTO_DOWNLOAD_NLTK, defaults to on,quiet=True). In thetest-unstructured2graphCI job this wasn't succeeding, and sincequiet=Truesuppressesnltk.download()'s own diagnostic output, the failure was silent — it only surfaced later as 22 unrelated-looking pytest failures (LookupErrordeep insideunstructured's text-partitioning path viapos_tag/contains_verb), not as a clear "download failed" error.Splits the previous single
Install dependencies and run testsstep into three: Install dependencies, Download NLTK data (new — explicitpython -m nltk.downloader averaged_perceptron_tagger_eng punkt_tab, noquietflag), and Run tests. If the download ever fails again, this step now fails loudly on its own instead of masquerading as broken test logic across 22 different test failures.What I couldn't pin down: the exact reason the silent auto-download wasn't succeeding in CI specifically. I verified the download mechanism itself works fine given a real network path — reproduced it locally against a fully isolated
HOME(no pre-existingnltk_data, matching CI's fresh-runner conditions) and it succeeded cleanly. So this doesn't look like a fundamental incompatibility with thenltk<3.10pin from #251, more likely a runner-specific network/timing hiccup. Either way, the actual problem — a silent failure surfacing as 22 confusing, unrelated-looking test failures — is fixed regardless of the exact CI-side trigger, and any future recurrence will now fail loudly on its own step.Test plan
uv venv→uv pip install -e .[test]→ download →pytest .) in a from-scratch venv with an isolatedHOME, faithfully simulating a fresh runner with no cached NLTK data.test-unstructured2graphjob's steps.