krickert commented on PR #2961: URL: https://github.com/apache/tika/pull/2961#issuecomment-6369389865
I went over the copilot review and issued the fixes Here's a summary, followed by the more detailed summary produced by AI: * Fixed the booleans fall back to string. * Fixed the edge case of a skip vs failed and updated the javadoc * Better error handling when paring closed early AI specifics: > Addressed the Copilot review in ffe0add7: > - **Unrecognized booleans fall back to strings** (`MetadataTagger`). `parseBoolean` now returns `null` for anything other than true/false, yes/no or 1/0 (any case). `tagByDeclaredType(BOOLEAN, ...)` then returns `null`, so the whole entry falls back to strings, as integers, numbers and dates already do, instead of arriving as a guessed `false`. Tests: `booleanParsingAcceptsCommonSpellings` now also checks that `maybe` and `N/A` give `null`, and the new `unrecognizedBooleanFallsBackToStrings` covers the fallback. > - **`PARSE_EXCEPTION_NO_EMIT` maps to `FAILED`** (`DocumentBuilder`). tika-grpc sends `ON_PARSE_EXCEPTION.SKIP`, so this status means the parse threw. It comes back with only the stack trace and no metadata list, so there is nothing partial to return. The Javadoc is updated, and the new `parseExceptionWithNothingEmittedIsFailedNotPartial` covers it. > - **An interrupted FetchAndParse closes the call with `UNAVAILABLE`** (`TikaGrpcServerImpl`, `TikaGrpcV2ServerImpl`). When `runFetchAndParse` is interrupted it has no outcome. The unary and server-streaming calls used to complete anyway with no reply, and a unary client saw `INTERNAL: No value received for unary call`. `fetchAndParseImpl` now closes the call through a shared `closeInterrupted` and returns `false`, and the caller skips `onCompleted()`. This applies to v1 and v2, and to the bidi streams. The new `testInterruptedFetchAndParseClosesWithUnavailable` interrupts the handler thread before the call (in-process transport, direct executor) and checks that v1 and v2 both send no reply, no completion and one `UNAVAILABLE` error. > Tested with `./mvnw test -pl tika-grpc-mapper,tika-grpc`: 33/33 and 29/29 pass, and checkstyle is clean. -- 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]
