kfaraz commented on code in PR #14614: URL: https://github.com/apache/druid/pull/14614#discussion_r1269007985
########## processing/src/main/java/org/apache/druid/data/input/InputSourceBuilder.java: ########## @@ -0,0 +1,44 @@ +/* + * 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.druid.data.input; + +import com.fasterxml.jackson.annotation.JsonSubTypes; +import com.fasterxml.jackson.annotation.JsonTypeInfo; +import org.apache.druid.data.input.impl.LocalInputSourceBuilder; +import org.apache.druid.data.input.impl.SplittableInputSource; +import org.apache.druid.guice.annotations.UnstableApi; + +import java.util.List; + +/** + * An interface to generate a {@link SplittableInputSource} objects on the fly. + * For composing input sources such as IcebergInputSource, the delegate input source instantiation might fail upon deserialization since the input file paths Review Comment: Re-reading this part of the comment, I think it is very implementation-specific and shouldn't really be included in the javadoc of this interface. ########## processing/src/main/java/org/apache/druid/data/input/InputSourceBuilder.java: ########## @@ -0,0 +1,44 @@ +/* + * 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.druid.data.input; + +import com.fasterxml.jackson.annotation.JsonSubTypes; +import com.fasterxml.jackson.annotation.JsonTypeInfo; +import org.apache.druid.data.input.impl.LocalInputSourceBuilder; +import org.apache.druid.data.input.impl.SplittableInputSource; +import org.apache.druid.guice.annotations.UnstableApi; + +import java.util.List; + +/** + * An interface to generate a {@link SplittableInputSource} objects on the fly. + * For composing input sources such as IcebergInputSource, the delegate input source instantiation might fail upon deserialization since the input file paths + * are not available yet and this might fail the input source precondition checks. + * This adapter helps create the delegate input source once the input file paths are fully determined. + */ +@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "type") Review Comment: Side comment: Requiring a builder interface to be serializable feels weird. From what I see in the code, it seems that this was done just so that we could bind named impls such as `S3InputSourceBuilder` and `HdfsInputSourceBuilder`. I guess another way to do this would have been to write providers for the input source builder impls in the respective modules. Or maybe there is some other cleaner approach. But we need not tackle this here. -- 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]
