williamhyun commented on PR #17457:
URL: https://github.com/apache/iceberg/pull/17457#issuecomment-5222161292

   Thank you @steveloughran that's a helpful way to scope it!
   I've bumped the block size to be 8MB and kept the HTTP response handling 
changes concentrated on a single easy-to-find seam rather than attempting full 
error classification here. Would appreciate if you took another look at it!
   
   To summarize what changed:
   * Added a `HttpStatusCategory` in core to map status code to a 
vendor-agnostic category such that call sites (sequential/positional reads, 
getLength(), and exists()) classify through it and then apply their own policy.
   
   * Coarse mapping for now: 
       - 403 -> terminal `ForbiddenException` on the read/length paths 
(exists() returns false [1])
       - 404 -> NotFoundException
       - 416 -> empty read
       - 5xx -> retried as transient on the read path (the one-shot length 
probe and exists() don't retry beyond Apache HttpClient's own default retry)
       - Anything else -> terminal
   Response-body parsing, store-specific throttle detection, and 
Retry-After/backoff have deliberately been left out to keep the PR minimal. 
   
   Thanks for the great context on throttling 403s. Since the pre-signed path 
bypasses the SDK, its brief internal retries don't apply here; but because the 
response handler already has the whole response in scope, a store-aware refiner 
(e.g. in the aws module) can later read the body and reclassify a throttling 
403 as transient as follow-up work, without touching these core call sites.
   
   [1] `exists()` classifies the same way but its policy reduces every non-2xx 
category to false instead of throwing. A cleaner future policy might let 
`exists()` distinguish between "definitely absent" (NOT_FOUND → false) and 
"couldn't verify" (FORBIDDEN/SERVER_ERROR → throw)


-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to