goutamadwant commented on code in PR #12281: URL: https://github.com/apache/seatunnel/pull/12281#discussion_r4002922472
########## seatunnel-connectors-v2/connector-http/connector-http-splunk/src/main/java/org/apache/seatunnel/connectors/seatunnel/splunk/config/SplunkSourceParameter.java: ########## @@ -0,0 +1,53 @@ +/* + * 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.seatunnel.connectors.seatunnel.splunk.config; + +import org.apache.seatunnel.api.configuration.ReadonlyConfig; +import org.apache.seatunnel.connectors.seatunnel.http.config.HttpParameter; + +import java.util.HashMap; + +public class SplunkSourceParameter extends HttpParameter { + /** + * Overrides buildWithConfig to accept an explicit apiKey parameter. Splunk's REST API requires + * the API key to be passed specifically as an Authorization header, so this method ensures the + * key is properly extracted and configured. + */ + public void buildWithConfig(ReadonlyConfig pluginConfig, String apiKey) { + super.buildWithConfig(pluginConfig); Review Comment: Please give this connector a real POST default after loading the shared HTTP options. `HttpSourceOptions.METHOD` defaults to GET, so omitting the optional `method`—whose documented default is POST—leaves `parameter.getMethod()` as `get`. Splunk's `/services/search/v2/jobs/export` endpoint does not provide GET, so that otherwise valid configuration fails. Please set or validate POST and add a request-capture test with `method` omitted. ########## docs/en/connectors/source/Http-Splunk.md: ########## @@ -0,0 +1,43 @@ +import ChangeLog from '../changelog/connector-http-splunk.md'; + +# Http-Splunk Source Connector + +## Description + +The `Http-Splunk` connector allows batch reading data from Splunk REST API endpoints using an HTTP source pattern. It utilizes token-based authentication and supports synchronous search export endpoints via POST requests. + +## Key Features + +* HTTP-based ingestion from Splunk REST API (`/services/search/v2/jobs/export`) +* Token-based authentication via the `Authorization` header +* Form-urlencoded parameter mapping for search queries and output formats + +## Options + +| Name | Type | Required | Default | Description | +| --- | --- | --- | --- | --- | +| url | String | Yes | - | Splunk REST API search export URL (`/services/search/v2/jobs/export`) | +| api_key | String | Yes | - | Splunk authentication token (e.g., `Splunk <token>`) | +| method | String | No | `POST` | HTTP request method | +| keep_params_as_form | Boolean | No | `true` | Keep parameters as form urlencoded | +| params | Map | Yes | - | Request parameters including `search` and `output_mode` | + +## Example Configuration + +```hocon +source { + Http-Splunk { Review Comment: Please use `Splunk {` here. SeaTunnel copies this block name verbatim into `plugin_name`; the factory and plugin mapping are both registered as `Splunk`, not `Http-Splunk`. I parsed this example through the current config parser and factory discovery failed with “Could not find any factory for identifier 'Http-Splunk'” (available identifiers: `Http`, `Splunk`), so the documented job cannot start. ########## seatunnel-connectors-v2/connector-http/connector-http-splunk/src/main/java/org/apache/seatunnel/connectors/seatunnel/splunk/SplunkSourceFactory.java: ########## @@ -0,0 +1,50 @@ +/* + * 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.seatunnel.connectors.seatunnel.splunk; + +import org.apache.seatunnel.api.configuration.util.OptionRule; +import org.apache.seatunnel.api.source.SeaTunnelSource; +import org.apache.seatunnel.api.source.SourceSplit; +import org.apache.seatunnel.api.table.connector.TableSource; +import org.apache.seatunnel.api.table.factory.Factory; +import org.apache.seatunnel.api.table.factory.TableSourceFactoryContext; +import org.apache.seatunnel.connectors.seatunnel.http.source.HttpSourceFactory; +import org.apache.seatunnel.connectors.seatunnel.splunk.config.SplunkSourceOptions; + +import com.google.auto.service.AutoService; + +import java.io.Serializable; + +@AutoService(Factory.class) +public class SplunkSourceFactory extends HttpSourceFactory { + @Override + public String factoryIdentifier() { + return "Splunk"; + } + + @Override + public <T, SplitT extends SourceSplit, StateT extends Serializable> + TableSource<T, SplitT, StateT> createSource(TableSourceFactoryContext context) { + return () -> (SeaTunnelSource<T, SplitT, StateT>) new SplunkSource(context.getOptions()); + } + + @Override + public OptionRule optionRule() { + return getHttpBuilder().required(SplunkSourceOptions.API_KEY).build(); Review Comment: Please declare `SplunkSourceOptions.KEEP_PARAMS_AS_FORM` as an optional factory option. It is documented and consumed by `SplunkSourceParameter`, but `getHttpBuilder()` does not include this key. I passed the documented configuration to `ConfigValidator.validateUnknownKeys`, used by SeaTunnel's config-validation paths, and it failed with `unknown option keys: [keep_params_as_form]`. Please add it to the option rule and cover the documented configuration with a validation test. ########## seatunnel-connectors-v2/connector-http/connector-http-splunk/src/main/java/org/apache/seatunnel/connectors/seatunnel/splunk/SplunkSource.java: ########## @@ -0,0 +1,56 @@ +/* + * 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.seatunnel.connectors.seatunnel.splunk; + +import org.apache.seatunnel.api.configuration.ReadonlyConfig; +import org.apache.seatunnel.api.table.type.SeaTunnelRow; +import org.apache.seatunnel.connectors.seatunnel.common.source.AbstractSingleSplitReader; +import org.apache.seatunnel.connectors.seatunnel.common.source.SingleSplitReaderContext; +import org.apache.seatunnel.connectors.seatunnel.http.source.HttpSource; +import org.apache.seatunnel.connectors.seatunnel.http.source.HttpSourceReader; +import org.apache.seatunnel.connectors.seatunnel.splunk.config.SplunkSourceOptions; +import org.apache.seatunnel.connectors.seatunnel.splunk.config.SplunkSourceParameter; + +import lombok.extern.slf4j.Slf4j; + +@Slf4j +public class SplunkSource extends HttpSource { + private final SplunkSourceParameter splunkSourceParameter = new SplunkSourceParameter(); + + public SplunkSource(ReadonlyConfig pluginConfig) { + super(pluginConfig); + String apiKey = pluginConfig.get(SplunkSourceOptions.API_KEY); + splunkSourceParameter.buildWithConfig(pluginConfig, apiKey); + } + + @Override + public String getPluginName() { + return "splunk"; + } + + @Override + public AbstractSingleSplitReader<SeaTunnelRow> createReader( + SingleSplitReaderContext readerContext) throws Exception { + return new HttpSourceReader( Review Comment: The row splitting and preview filtering are fixed, but the incremental part of this thread is still unresolved. `super.executeRequest()` still buffers the complete export through `EntityUtils.toString()`, and `filterAndUnwrapNdjson()` then calls `split()` and builds a second full response string before the parent reads it line by line. I reproduced this at the current head with a 30,000,148-byte valid Splunk NDJSON response under a 96 MiB heap: the raw response fit, but `filterAndUnwrapNdjson()` threw `OutOfMemoryError`. Please process the response entity line by line and emit or unwrap rows without materializing both the complete raw and filtered responses, or use the job/results pagination flow for bounded memory. -- 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]
