abhinav-phi commented on PR #2118:
URL: https://github.com/apache/stormcrawler/pull/2118#issuecomment-5574140084
Thank you both for the review — all findings addressed in a0ab40e8 (rebased
onto current `main`):
**@tballison's bot findings**
1. **`parse-context` silently ignored** — fixed. The bolt now keeps the
`ParseContext` from `TikaLoader.loadParseContext()` and seeds every per-parse
context with `copyFrom()` (each `execute()` gets its own copy, since bolts are
multi-threaded). Covered by `TikaConfigTest#testParseContextSeededFromConfig`,
which loads a config with `"parse-context": {"output-limits": {"writeLimit":
100000}}` and asserts `OutputLimits.get(context).getWriteLimit() == 100000`
(and `UNLIMITED` on an unseeded context). I did not add the
truncation-through-parse test you sketched: at 4.0.0 `AutoDetectParser`'s
default `ContentHandlerDecoratorFactory` is a no-op, so a caller-supplied
`BodyContentHandler` bypasses the `WriteOutContentHandler` that would enforce
`writeLimit` — the limit reaches `SecureContentHandler` (zip-bomb/depth) but
not text truncation unless the user configures a handler-decorating factory.
That is a Tika-side gap worth a punt-list mention rather than something to work
around in
StormCrawler.
2. **Broken config degrades silently to defaults (OCR on)** — fixed by
failing fast: `instantiateTika()` now throws from `prepare()` when the resource
is missing or `TikaLoader.load()` fails. Agreed that for a crawler a failed
deploy beats a silent config downgrade. Covered by
`TikaConfigTest#testBrokenConfigFailsFast` (unknown key → Tika 4's mapper
rejects it) and `#testMissingConfigFailsFast`. The OCR exclusion itself is now
*proven*, not assumed: `#testTesseractOCRExcludedByDefaultConfig` walks
`CompositeParser.getAllComponentParsers()` and asserts no `TesseractOCRParser`.
3. **Temp copy kept for worker lifetime** — fixed: deleted in a `finally`
right after `TikaLoader.load()` returns (the config is read eagerly),
warning-only if the delete fails. Agree the InputStream-shaped hole in
`TikaLoader` is a Tika gap for the punt list.
**Subtlety on `extractEmbedded`** — confirmed and fixed: it was already
broken in 3.x. `AutoDetectParser.initializeEmbeddedParserAndDetector()`
self-binds when `Parser.class` is absent from the context, so embedded content
was parsed despite the setting. The bolt now binds `EmptyParser.INSTANCE` when
`!extractEmbedded` (mirroring `DelegatingParser`'s own fallback), and
`ParserBoltTest#testEmbeddedNotParsedByDefault` pins it: the recursive embedded
`.docx` no longer leaks the embedded documents' text into the parent tuple.
**Hygiene**
- Tika metadata clue calls now use `org.apache.tika.metadata.HttpHeaders`
(in both bolts); StormCrawler `Metadata` lookups keep the String-keyed
`org.apache.http.HttpHeaders` since those address our own key space, not Tika's.
- README link → https://tika.apache.org/docs/4.0.x/configuration/index.html.
- Archetype yamls corrected to `parser.tika.config.file` (the `parse.`
variant was never read — pre-existing, but fixed since the PR touches those
lines). Docs wording now says classpath resource.
- PR description updated: the dropped `service-loader` handlers are
described as inert in 4.0 (the section deserializes into an empty POJO), not
"the defaults".
**Licences (rzo1)** — checked the actual jars: `tika-ml-junkdetect`,
`tika-ml-core` and `tika-encoding-detector-mojibuster` are Apache Tika (ASF)
artifacts, Apache License 2.0 per their bundled `META-INF/LICENSE`/`NOTICE`.
Their only third-party runtime dependency is `org.commonmark:commonmark`
(BSD-2-Clause, already an approved category in `THIRD-PARTY.txt`). The bundled
model resources (`junkdetect.bin`, `nb-bigram.bin`) are ASF-generated and
RAT-excluded by Tika's own build. Details added to the PR description.
**On `PipesForkParser`** — agreed it's the right tool for process isolation,
but out of scope for this PR; noted in the description alongside the
(bot-corrected) point that timeout configs belong to the pipes/forked path, not
our in-process parsing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
