singhpk234 commented on code in PR #17457: URL: https://github.com/apache/iceberg/pull/17457#discussion_r3920255837
########## core/src/main/java/org/apache/iceberg/io/http/HttpStatusCategory.java: ########## @@ -0,0 +1,66 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.io.http; + +import org.apache.hc.core5.http.HttpStatus; + +/** + * Coarse, vendor-agnostic classification of an HTTP status code for the pre-signed-URL read path. + * + * <p>This is the single place that maps a raw status code to a category; call sites choose the + * action for each one (read the body, treat as end-of-file, retry, or fail). Only {@link + * #TRANSIENT} is retried by the read path. Not every {@code 5xx} is transient: an idempotent {@code + * GET} is only retried on throttling ({@code 429}) and the server errors that are safe to replay + * ({@code 408}, {@code 500}, {@code 502}, {@code 503}, {@code 504}), matching the retryable codes + * used by {@code ExponentialHttpRequestRetryStrategy}. Every other non-success status (including + * {@code 404}, {@code 403}, {@code 412}, and server errors such as {@code 501}) is {@link + * #TERMINAL} and fails without a retry. + * + * <p>Concentrating the mapping here keeps the response-handling seam easy to find, so a + * store-specific step can later inspect the response body and reclassify an otherwise-terminal + * status that is actually transient (for example a throttling {@code 403}, which some object stores + * return under heavy load) so that it is retried too. + */ +enum HttpStatusCategory { Review Comment: This is doing a classification of the error code to states ... for example which as transient (throttling) / which are persistent (403..) and based on that its taking action in stream .... i wonder why can't we do this in the https://github.com/apache/iceberg/pull/17457/changes#diff-88dbd57947faabb967a76beac0e51b21b52df326a25086cde68f983c43256708R238 ? will think this more -- 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]
