bamaer commented on PR #8716:
URL: https://github.com/apache/hop/pull/8716#issuecomment-5946687718
**Blockers**
- **Add Sequence fails on a real Hop install.** The new
`BeamAddSequenceTransformHandler` creates `AddSequenceMeta`, but
`plugins/engines/beam/src/main/resources/dependencies.xml` has no
`../../transforms/addsequence` entry. Every Beam pipeline with Add Sequence
fails with `NoClassDefFoundError: AddSequenceMeta`, including the existing
`beam_directrunner/main-0003` integration test. Unit tests and CI don't catch
it: Maven puts all plugins on one classpath, and CI doesn't run the integration
tests. Fix: add `<folder>../../transforms/addsequence</folder>`. With that
line, all 15 `beam_directrunner` integration tests pass.
**Should fix**
- **Add Sequence on Beam regresses three cases that worked in 2.18.** The
new handler does fix duplicate values in batch pipelines: 2.18 gave 582
distinct values for 1000 rows, this PR gives 1000. But it breaks three cases
that worked in 2.18:
- Unbounded input (Kafka, MQTT, Debezium, a never-ending Row Generator)
now fails with `GroupByKey cannot be applied to non-bounded PCollection in the
GlobalWindow without a trigger`.
- "Use DB to get sequence" is refused
(`BeamAddSequenceTransformHandler.java:103`).
- Rows above the max value are dropped without an error
(`AddSequenceFn.java:125`). With max 10 and 30 rows in, 10 rows come out; 2.18
and the local engine wrap around.
- Fix: use the generic handler for DB mode and unbounded input, and wrap
around like `Counter`.
- **The keyed window ignores the trigger, allowed lateness and discarding
mode** (`BeamWindowMeta.java:313`). The keyed path rebuilds the window from
`getWindowFn()` only. A Global window with a trigger on an unbounded input
works without a key, but fails with the GroupByKey error above once a key field
is set. Fix: apply the same settings to `keyedWindow`.
- **Compressed file output doubles the extension.** With suffix `.csv.gz`,
GZIP and AUTO both write `rows-00000-of-00003.csv.gz.gz`
(`BeamOutputTransform.java:116,139`). The docs tell users to put `.gz` in the
suffix for AUTO, so AUTO always doubles it. Fix: strip the codec's suffix
before `withSuffix`, and assert file names in the test.
- **The new dependencies add 114 MB to `lib/core` for everyone who installs
Beam.**
- 96.5 MB of that is `snowflake-jdbc-4.3.0`. It contains its own unshaded
copy of zstd-jni, and with Beam installed Hop's Parquet ZSTD codec loads
`com.github.luben.zstd.Zstd` from the Snowflake jar instead of
`zstd-jni-1.5.7-13.jar`. It works, but which copy wins depends on classpath
order.
- `beam-sdks-java-io-parquet` (pom.xml:1176) is compile scope, but only
`ParquetIOViabilityTest` uses it, so about 10 MB of Parquet jars ship unused.
Make it test scope.
- Consider whether SnowflakeIO belongs in the default Beam plugin.
- **The Beam partition transform has no effect.**
`BeamPartitionMeta.java:215` flattens the partitions back into one stream, so a
following GroupByKey still shuffles. `beampartition.adoc` claims it avoids that
shuffle and that "single partition serialises processing". Fix the docs, or
give each partition its own output.
- **LICENSE (line 520) and NOTICE (line 52) list `bcprov-jdk15on`, but the
pom excludes it** and it's not in the zip. Remove those entries; the NOTICE
paragraph about licenses belongs in LICENSE.
- **`issue-8708-triage.adoc` is project tracking in the user manual**, with
commit hashes that won't survive a squash, and it's linked from `nav.adoc:70`.
Move it to the umbrella issue.
*Verified: built with all unit tests; ran the `beam_directrunner`
integration tests; ran pipelines on the Direct runner against live MQTT,
Elasticsearch 8, Splunk 9 HEC and Postgres/Debezium; compared Add Sequence
behaviour with Hop 2.18. Not run: Snowflake, BigQuery, remote runners.*
--
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]
