xiangfu0 commented on code in PR #20067: URL: https://github.com/apache/pinot/pull/20067#discussion_r4457401842
########## pinot-spi/src/main/java/org/apache/pinot/spi/ingestion/error/IngestionSourceErrorEvidence.java: ########## @@ -0,0 +1,165 @@ +/** + * 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.pinot.spi.ingestion.error; + +import com.google.common.collect.MapMaker; +import java.net.URI; +import java.net.URISyntaxException; +import java.util.concurrent.ConcurrentMap; +import javax.annotation.Nullable; + + +public final class IngestionSourceErrorEvidence extends RuntimeException { + private static final long serialVersionUID = 1L; + + public static final String UNREACHABLE = "unreachable"; + public static final String LOCAL_PRECHECK = "local_precheck"; + public static final String UNKNOWN_TOPIC_OR_PARTITION = "UNKNOWN_TOPIC_OR_PARTITION"; + + private static final ConcurrentMap<Throwable, IngestionSourceErrorEvidence> UNSUPPRESSIBLE = + new MapMaker().weakKeys().makeMap(); + + @Nullable + private String _target; + private int _status; + @Nullable + private String _code; + @Nullable + private String _resource; + private boolean _boundary; + + public IngestionSourceErrorEvidence() { + super(null, null, false, false); + } + + public static <T extends Throwable> T attach(T failure, IngestionSourceErrorEvidence evidence) { Review Comment: Related: the warning that `attach` must not be used on a shared exception instance (Kafka's `Errors.X.exception()`) is gone. Attaching evidence to such an instance would affect every later failure that reuses it, and no test can catch that misuse in a caller. This is the kind of contract that belongs in the Javadoc, as does the rule that evidence must never hold message text. ########## pinot-spi/src/main/java/org/apache/pinot/spi/ingestion/error/IngestionSourceErrorRule.java: ########## @@ -0,0 +1,58 @@ +/** + * 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.pinot.spi.ingestion.error; + +import java.lang.reflect.Method; +import java.util.List; +import javax.annotation.Nullable; + + +@FunctionalInterface +public interface IngestionSourceErrorRule { Review Comment: **Major (CLAUDE.md convention):** this public extension point no longer has any Javadoc. A plugin author can't tell from the code that returning `none(...)` is different from returning `null`, what order `below` is in, or that SDK classes must be matched with `hasClass` rather than `instanceof`. CLAUDE.md asks for class-level Javadoc on new classes, describing behaviour and thread-safety. Could the public types (`IngestionSourceErrorClassifier`, `IngestionSourceErrorRule`, `IngestionSourceErrorEvidence`, `IngestionSourceErrorVerdict`, `IngestionSourceErrorClassification`, `IngestionSourceErrorCategory`) get back at least their class and contract docs, even if the comments explaining internal rules stay out? -- 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]
