mattcasters commented on code in PR #8786:
URL: https://github.com/apache/hop/pull/8786#discussion_r4210139744


##########
plugins/transforms/serverstatus/src/main/samples/transforms/get-server-status.hpl:
##########
@@ -0,0 +1,92 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+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.
+
+-->
+<pipeline>
+  <info>
+    <name>get-server-status</name>
+    <name_sync_with_filename>Y</name_sync_with_filename>
+    <description/>
+    <extended_description/>
+    <pipeline_version/>
+    <pipeline_type>Normal</pipeline_type>
+    <parameters>
+    </parameters>
+    <capture_transform_performance>N</capture_transform_performance>
+    
<transform_performance_capturing_delay>1000</transform_performance_capturing_delay>
+    
<transform_performance_capturing_size_limit>100</transform_performance_capturing_size_limit>
+    <created_user>-</created_user>
+    <created_date>2026/10/04 09:00:00.000</created_date>
+    <modified_user>-</modified_user>
+    <modified_date>2026/10/04 09:00:00.000</modified_date>
+  </info>
+  <notepads>
+    <notepad>
+      <note>Query the status of a Hop server. Requires a reachable Hop 
server.</note>
+      <xloc>128</xloc>
+      <yloc>16</yloc>
+      <width>400</width>
+      <heigth>27</heigth>
+      <fontname>Noto Sans</fontname>
+      <fontsize>11</fontsize>
+      <fontbold>N</fontbold>
+      <fontitalic>N</fontitalic>
+      <fontcolorred>14</fontcolorred>
+      <fontcolorgreen>58</fontcolorgreen>
+      <fontcolorblue>90</fontcolorblue>
+      <backgroundcolorred>201</backgroundcolorred>
+      <backgroundcolorgreen>232</backgroundcolorgreen>
+      <backgroundcolorblue>251</backgroundcolorblue>
+      <bordercolorred>14</bordercolorred>
+      <bordercolorgreen>58</bordercolorgreen>
+      <bordercolorblue>90</bordercolorblue>
+    </notepad>
+  </notepads>
+  <order>
+  </order>
+<transform>
+    <name>get server status</name>
+    <type>GetServerStatus</type>
+    <description/>
+    <distribute>N</distribute>
+    <custom_distribution/>
+    <copies>1</copies>
+    <partitioning>
+      <method>none</method>
+      <schema_name/>
+    </partitioning>
+    <server_field>serverName</server_field>

Review Comment:
   **[bug]** `GetServerStatus` only reads `server_field` after an input row 
arrives. This pipeline has no upstream transform, so `processRow()` gets a null 
row and finishes before it looks up `serverName`. The note says a reachable Hop 
server is required, but nothing is queried, and the sample is not on the 
integration-test ignore list.
   
   **Suggestion:** Feed a server name from a data grid, and either ship a 
matching Hop-server metadata object or add `get-server-status.hpl` to the 
ignore list so a missing server does not fail `read-samples-build-hop-run`.



##########
plugins/transforms/dbimpact/src/main/samples/transforms/database-impact.hpl:
##########
@@ -0,0 +1,119 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+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.
+
+-->
+<pipeline>
+  <info>
+    <name>database-impact</name>
+    <name_sync_with_filename>Y</name_sync_with_filename>
+    <description/>
+    <extended_description/>
+    <pipeline_version/>
+    <pipeline_type>Normal</pipeline_type>
+    <parameters>
+    </parameters>
+    <capture_transform_performance>N</capture_transform_performance>
+    
<transform_performance_capturing_delay>1000</transform_performance_capturing_delay>
+    
<transform_performance_capturing_size_limit>100</transform_performance_capturing_size_limit>
+    <created_user>-</created_user>
+    <created_date>2026/10/04 09:00:00.000</created_date>
+    <modified_user>-</modified_user>
+    <modified_date>2026/10/04 09:00:00.000</modified_date>
+  </info>
+  <notepads>
+    <notepad>
+      <note>Collect the database impact of the transforms in a pipeline.</note>
+      <xloc>128</xloc>
+      <yloc>16</yloc>
+      <width>400</width>
+      <heigth>27</heigth>
+      <fontname>Noto Sans</fontname>
+      <fontsize>11</fontsize>
+      <fontbold>N</fontbold>
+      <fontitalic>N</fontitalic>
+      <fontcolorred>14</fontcolorred>
+      <fontcolorgreen>58</fontcolorgreen>
+      <fontcolorblue>90</fontcolorblue>
+      <backgroundcolorred>201</backgroundcolorred>
+      <backgroundcolorgreen>232</backgroundcolorgreen>
+      <backgroundcolorblue>251</backgroundcolorblue>
+      <bordercolorred>14</bordercolorred>
+      <bordercolorgreen>58</bordercolorgreen>
+      <bordercolorblue>90</bordercolorblue>
+    </notepad>
+  </notepads>
+  <order>
+    <hop>
+      <from>rows</from>
+      <to>database impact input</to>
+      <enabled>Y</enabled>
+    </hop>
+  </order>
+<transform>
+    <name>rows</name>
+    <type>DataGrid</type>
+    <description/>
+    <distribute>Y</distribute>
+    <custom_distribution/>
+    <copies>1</copies>
+    <partitioning>
+      <method>none</method>
+      <schema_name/>
+    </partitioning>
+    <fields>
+      <field>
+        <name>value</name>
+        <type>String</type>
+        <format></format>
+        <length>-1</length>
+        <precision>-1</precision>
+        <set_empty_string>N</set_empty_string>
+      </field>
+    </fields>
+    <data>
+      <line>
+        <item>sample-value</item>
+      </line>
+    </data>
+    <attributes/>
+    <GUI>
+      <xloc>128</xloc>
+      <yloc>160</yloc>
+    </GUI>
+  </transform>
+  <transform>
+    <name>database impact input</name>
+    <type>DbImpactInput</type>

Review Comment:
   **[bug]** Database impact input only analyses a pipeline when 
`filename-field` names a field that holds a pipeline or workflow path. This 
transform never sets that property, and the only incoming value is the literal 
`sample-value`, so `DbImpactInput` skips the row and writes nothing. The note 
says the sample collects impact, but a run produces no impact rows.
   
   **Suggestion:** Add a filename field, for example 
`${PROJECT_HOME}/transforms/table-exists.hpl`, and set `<filename-field>` to 
that field.



##########
integration-tests/samples/read-samples-build-hop-run.hpl:
##########
@@ -575,6 +583,274 @@ var hop_run_cmd = './hop-run.sh -j samples -r ' + 
run_config + ' -f ' + filename
         <item>structured-extract-basic.hpl</item>
         <item>requires an AI provider named ollama-local</item>
       </line>
+      <line>
+        <item>call-db-procedure.hpl</item>
+        <item>requires a stored procedure in the database</item>
+      </line>
+      <line>
+        <item>get-table-names.hpl</item>
+        <item>fails on the H2 2.x metadata of the hop-samples connection 
(INFORMATION_SCHEMA.CONSTANTS)</item>
+      </line>
+      <line>
+        <item>http-client.hpl</item>
+        <item>requires network access to the URL</item>
+      </line>
+      <line>
+        <item>http-post.hpl</item>
+        <item>requires network access to the URL</item>
+      </line>
+      <line>
+        <item>web-services-lookup.hpl</item>
+        <item>requires network access to the web service</item>
+      </line>
+      <line>
+        <item>mail.hpl</item>
+        <item>requires mail connection sample-mail</item>
+      </line>
+      <line>
+        <item>email-messages-input.hpl</item>
+        <item>requires mail connection sample-mail</item>
+      </line>
+      <line>
+        <item>ldap-input.hpl</item>
+        <item>requires reachable LDAP server</item>
+      </line>
+      <line>
+        <item>ldap-output.hpl</item>
+        <item>requires reachable LDAP server</item>
+      </line>
+      <line>
+        <item>jms-consumer.hpl</item>
+        <item>requires JMS connection sample-jms</item>
+      </line>
+      <line>
+        <item>jms-producer.hpl</item>
+        <item>requires JMS connection sample-jms</item>
+      </line>
+      <line>
+        <item>splunk-input.hpl</item>
+        <item>requires reachable Splunk server</item>
+      </line>
+      <line>
+        <item>execution-information.hpl</item>
+        <item>requires execution info location metadata</item>
+      </line>
+      <line>
+        <item>sas-input.hpl</item>
+        <item>requires a SAS data set file</item>
+      </line>
+      <line>
+        <item>pgp-encrypt-stream.hpl</item>
+        <item>requires gpg and the PGP key of the recipient</item>
+      </line>
+      <line>
+        <item>pgp-decrypt-stream.hpl</item>
+        <item>requires gpg and a PGP private key</item>
+      </line>
+      <line>
+        <item>dataset-input.hpl</item>
+        <item>requires unit test data set metadata</item>
+      </line>
+      <line>
+        <item>dataset-output.hpl</item>
+        <item>requires unit test data set metadata</item>
+      </line>
+      <line>
+        <item>execute-unit-tests.hpl</item>
+        <item>requires unit test metadata</item>
+      </line>
+      <line>
+        <item>aws-sns-notify.hpl</item>
+        <item>requires AWS credentials</item>
+      </line>
+      <line>
+        <item>aws-sqs-reader.hpl</item>
+        <item>requires AWS credentials</item>
+      </line>
+      <line>
+        <item>redshift-bulk-loader.hpl</item>
+        <item>requires AWS credentials and a Redshift cluster</item>
+      </line>
+      <line>
+        <item>azure-event-hubs-listener.hpl</item>
+        <item>requires Azure Event Hubs and credentials</item>
+      </line>
+      <line>
+        <item>azure-event-hubs-writer.hpl</item>
+        <item>requires Azure Event Hubs and credentials</item>
+      </line>
+      <line>
+        <item>cassandra-input.hpl</item>
+        <item>requires reachable Cassandra cluster</item>
+      </line>
+      <line>
+        <item>cassandra-output.hpl</item>
+        <item>requires reachable Cassandra cluster</item>
+      </line>
+      <line>
+        <item>cassandra-sstable-output.hpl</item>
+        <item>requires Cassandra connection settings</item>
+      </line>
+      <line>
+        <item>google-sheets-input.hpl</item>
+        <item>requires Google credentials</item>
+      </line>
+      <line>
+        <item>google-sheets-output.hpl</item>
+        <item>requires Google credentials</item>
+      </line>
+      <line>
+        <item>google-analytics-4.hpl</item>
+        <item>requires Google credentials</item>
+      </line>
+      <line>
+        <item>salesforce-input.hpl</item>
+        <item>requires Salesforce credentials</item>
+      </line>
+      <line>
+        <item>salesforce-insert.hpl</item>
+        <item>requires Salesforce credentials</item>
+      </line>
+      <line>
+        <item>salesforce-update.hpl</item>
+        <item>requires Salesforce credentials</item>
+      </line>
+      <line>
+        <item>salesforce-upsert.hpl</item>
+        <item>requires Salesforce credentials</item>
+      </line>
+      <line>
+        <item>salesforce-delete.hpl</item>
+        <item>requires Salesforce credentials</item>
+      </line>
+      <line>
+        <item>pgvector-search.hpl</item>
+        <item>requires PostgreSQL with the pgvector extension</item>
+      </line>
+      <line>
+        <item>pgvector-upsert.hpl</item>
+        <item>requires PostgreSQL with the pgvector extension</item>
+      </line>
+      <line>
+        <item>sftp-put.hpl</item>
+        <item>requires reachable SFTP host and credentials</item>
+      </line>
+      <line>
+        <item>embed-text.hpl</item>
+        <item>requires AI connection with an embedding model</item>
+      </line>
+      <line>
+        <item>mssql-bulk-loader.hpl</item>
+        <item>requires a SQL Server connection named mssql</item>
+      </line>
+      <line>
+        <item>mysql-bulk-loader.hpl</item>
+        <item>requires a MySQL connection named mysql</item>
+      </line>
+      <line>
+        <item>monetdb-bulk-loader.hpl</item>
+        <item>requires a MonetDB connection named monetdb</item>
+      </line>
+      <line>
+        <item>postgresql-bulk-loader.hpl</item>
+        <item>requires a PostgreSQL connection named postgresql</item>
+      </line>
+      <line>
+        <item>oracle-bulk-loader.hpl</item>
+        <item>requires an Oracle connection named oracle</item>
+      </line>
+      <line>
+        <item>cratedb-bulk-loader.hpl</item>
+        <item>requires a CrateDB connection named cratedb</item>
+      </line>
+      <line>
+        <item>snowflake-bulk-loader.hpl</item>
+        <item>requires a Snowflake connection named snowflake</item>
+      </line>
+      <line>
+        <item>microsoft-access-output.hpl</item>
+        <item>requires an Access database file and table name</item>
+      </line>
+      <line>
+        <item>doris-bulk-loader.hpl</item>
+        <item>requires a reachable Doris frontend</item>
+      </line>
+      <line>
+        <item>teradata-fastload-bulk-loader.hpl</item>
+        <item>requires a Teradata connection named teradata</item>
+      </line>
+      <line>
+        <item>rules-executor.hpl</item>
+        <item>requires a rules definition</item>
+      </line>
+      <line>
+        <item>beam-bigquery-input.hpl</item>
+        <item>requires Beam run configuration and GCP credentials</item>
+      </line>
+      <line>
+        <item>beam-bigquery-output.hpl</item>
+        <item>requires Beam run configuration and GCP credentials</item>
+      </line>
+      <line>
+        <item>beam-bigtable-input.hpl</item>
+        <item>requires Beam run configuration and GCP credentials</item>
+      </line>
+      <line>
+        <item>beam-gcp-pubsub-publish.hpl</item>
+        <item>requires Beam run configuration and GCP credentials</item>
+      </line>
+      <line>
+        <item>beam-gcp-pubsub-subscribe.hpl</item>
+        <item>requires Beam run configuration and GCP credentials</item>
+      </line>
+      <line>
+        <item>beam-kinesis-consume.hpl</item>
+        <item>requires Beam run configuration and AWS credentials</item>
+      </line>
+      <line>
+        <item>beam-kinesis-produce.hpl</item>
+        <item>requires Beam run configuration and AWS credentials</item>
+      </line>
+      <line>
+        <item>beam-timestamp.hpl</item>
+        <item>requires Beam run configuration</item>
+      </line>
+      <line>
+        <item>beam-hive-catalog-input.hpl</item>
+        <item>requires Beam run configuration and a Hive metastore</item>
+      </line>
+      <line>
+        <item>structured-extract-basic.hpl</item>

Review Comment:
   **[nit]** `structured-extract-basic.hpl` is already in this ignore list 
("requires an AI provider named ollama-local"). This second row only repeats 
the exclusion with a slightly different reason.
   
   **Suggestion:** Drop the duplicate row.



##########
plugins/transforms/pipelineexecutor/src/main/samples/transforms/pipeline-executor.hpl:
##########
@@ -0,0 +1,138 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+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.
+
+-->
+<pipeline>
+  <info>
+    <name>pipeline-executor</name>
+    <name_sync_with_filename>Y</name_sync_with_filename>
+    <description/>
+    <extended_description/>
+    <pipeline_version/>
+    <pipeline_type>Normal</pipeline_type>
+    <parameters>
+    </parameters>
+    <capture_transform_performance>N</capture_transform_performance>
+    
<transform_performance_capturing_delay>1000</transform_performance_capturing_delay>
+    
<transform_performance_capturing_size_limit>100</transform_performance_capturing_size_limit>
+    <created_user>-</created_user>
+    <created_date>2026/10/04 09:00:00.000</created_date>
+    <modified_user>-</modified_user>
+    <modified_date>2026/10/04 09:00:00.000</modified_date>
+  </info>
+  <notepads>
+    <notepad>
+      <note>Execute a child pipeline for every row and pass the rows of the 
child back as a data stream.</note>
+      <xloc>128</xloc>
+      <yloc>16</yloc>
+      <width>400</width>
+      <heigth>27</heigth>
+      <fontname>Noto Sans</fontname>
+      <fontsize>11</fontsize>
+      <fontbold>N</fontbold>
+      <fontitalic>N</fontitalic>
+      <fontcolorred>14</fontcolorred>
+      <fontcolorgreen>58</fontcolorgreen>
+      <fontcolorblue>90</fontcolorblue>
+      <backgroundcolorred>201</backgroundcolorred>
+      <backgroundcolorgreen>232</backgroundcolorgreen>
+      <backgroundcolorblue>251</backgroundcolorblue>
+      <bordercolorred>14</bordercolorred>
+      <bordercolorgreen>58</bordercolorgreen>
+      <bordercolorblue>90</bordercolorblue>
+    </notepad>
+  </notepads>
+  <order>
+    <hop>
+      <from>rows</from>
+      <to>pipeline executor</to>
+      <enabled>Y</enabled>
+    </hop>
+  </order>
+<transform>
+    <name>rows</name>
+    <type>DataGrid</type>
+    <description/>
+    <distribute>Y</distribute>
+    <custom_distribution/>
+    <copies>1</copies>
+    <partitioning>
+      <method>none</method>
+      <schema_name/>
+    </partitioning>
+    <fields>
+      <field>
+        <name>id</name>
+        <type>Integer</type>
+        <format></format>
+        <length>-1</length>
+        <precision>-1</precision>
+        <set_empty_string>N</set_empty_string>
+      </field>
+    </fields>
+    <data>
+      <line>
+        <item>1</item>
+      </line>
+      <line>
+        <item>2</item>
+      </line>
+      <line>
+        <item>3</item>
+      </line>
+    </data>
+    <attributes/>
+    <GUI>
+      <xloc>128</xloc>
+      <yloc>160</yloc>
+    </GUI>
+  </transform>
+  <transform>
+    <name>pipeline executor</name>
+    <type>PipelineExecutor</type>
+    <description/>
+    <distribute>Y</distribute>
+    <custom_distribution/>
+    <copies>1</copies>
+    <partitioning>
+      <method>none</method>
+      <schema_name/>
+    </partitioning>
+    <run_configuration>local</run_configuration>
+    <wait_timeout>0</wait_timeout>
+    <filename>${PROJECT_HOME}/transforms/pipeline-executor-child.hpl</filename>
+    <group_size>1</group_size>
+    <group_field/>
+    <group_time/>
+    <parameters/>
+    <inherit_all_vars>Y</inherit_all_vars>
+    <execution_result_target_transform/>

Review Comment:
   **[suggestion]** The note says the child rows are passed back as a data 
stream, but `execution_result_target_transform` is empty and there is no 
output-rows source transform. `PipelineExecutor` only copies child rows and 
execution statistics when those targets are set, so 
`pipeline-executor-child.hpl` runs and its `child_value` row is discarded.
   
   **Suggestion:** Hop the executor to a downstream transform, set the 
execution-result target, and set the output-rows source to `child data`. 
Otherwise change the note so it does not claim the child rows are returned.



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

Reply via email to