SEZ9 commented on issue #12707:
URL: https://github.com/apache/seatunnel/issues/12707#issuecomment-6098756136
Thanks for writing this up — the thin-launcher framing is right, and the
constraint list is unusually complete for a packaging proposal. I went through
the pieces it depends on in the current tree. Four of them check out, one does
not, and that one changes a requirement. Concrete split below; happy to take
the implementation.
## Verified against the tree
- **The checkpoint warning is real.** `config/seatunnel.yaml` lines 35-37
are `namespace: /tmp/seatunnel/checkpoint_snapshot` with `fs.defaultFS:
file:///tmp/`. A CDC job resumed after a temp sweep does lose its position, so
the launcher generating a per-user data directory is a correctness requirement,
not polish.
- **Checksum verification has in-tree precedent.** `bin/install-plugin.sh`
already verifies downloads and probes for `sha512sum` / `sha1sum` / `shasum`
before it starts. The launcher should mirror that probe rather than invent its
own.
- **The mirror / offline override has precedent too**:
`SEATUNNEL_PLUGIN_DOWNLOAD_METHOD` and `SEATUNNEL_MAVEN_REPOSITORY`. Worth
naming the launcher's tarball override consistently with those.
- **`tools/seatunnel-python-sdk` is distribution `seatunnel`** (v0.1,
`httpx`, `>=3.9`) — the one in-tree Python package already holding the name.
## One correction, and it matters
> Fail fast on Java below 11 with an actionable message (same check as
`seatunnel.sh`).
There is no such check to reuse.
`seatunnel-core/seatunnel-starter/src/main/bin/seatunnel.sh` ends in a bare
```
java ${JAVA_OPTS} -cp ${CLASS_PATH} ${APP_MAIN} "${args[@]}"
```
with no `JAVA_HOME` resolution and no version gate anywhere in the starter
scripts. On Java 8 the user currently gets an `UnsupportedClassVersionError`
straight out of the JVM, which is exactly the confusing failure this
requirement is trying to prevent.
So the launcher **introduces** this check rather than mirroring one. Still
the right call, and strictly better than today's behaviour — but it needs its
own design (parse `java -version`, honour `JAVA_HOME` ahead of `PATH`), and it
is worth considering whether `seatunnel.sh` should gain the same gate
separately.
## `install-plugin` needs a different shape than the sketch
> `seatunnel install-plugin jdbc cdc-mysql` -> update config/plugin_config,
run bin/install-plugin.sh
Right that `config/plugin_config` has to be updated first, but
`bin/install-plugin.sh` takes **a version** as `$1` (defaulting to `3.0.0`),
not plugin names — the plugin list comes only from `config/plugin_config`, a
flat list under `--connectors-v2--` section headers. So the launcher's job is:
rewrite that file down to the requested subset, then invoke `install-plugin.sh
<version>`. That rewrite has to be idempotent and must not drop the section
headers.
## Proposed split: the PMC decision gates publication, not implementation
Both open decisions are about *publishing*. Nothing in the download / verify
/ exec core depends on them, so I would split along that line:
**PR 1 — launcher core, no new name claimed.** Extend
`tools/seatunnel-python-sdk` rather than adding a directory. That satisfies "do
not add a third Python package" by construction, and means this PR needs no
naming decision at all:
- version resolution and tarball URL construction
- download, then **verify sha512 and the `.asc` against `KEYS` before
extracting**, reusing the hash-tool probe from `install-plugin.sh`
- per-version, per-user cache directory — explicitly *not* inside the
install prefix, since a wheel's `site-packages` is shared between users of the
interpreter and frequently not writable
- Java detection: `JAVA_HOME` then `PATH`, parse the version, actionable
message below 11
- a generated per-user config overriding the `/tmp` checkpoint namespace
- `install-plugin`: idempotent `plugin_config` rewrite plus
`install-plugin.sh <version>`
- local-mode run via subprocess; cluster submit reuses the SDK's existing
`jobs.submit_job`, so there is one REST path rather than two
**PR 2 — publication wiring**, once the name and release path are settled:
distribution name, console script, entry points. A thin shell over PR 1.
## Acceptance tests
The four from the issue, plus two the security surface needs:
- fresh virtualenv install, then `FakeSource -> Console` in local mode
- checksum mismatch aborts **without extracting**
- **signature mismatch aborts without extracting** (a separate path from
checksum)
- missing or old Java gives the actionable message
- second run works offline from the cache
- `plugin_config` rewrite is idempotent and preserves section headers
## Open questions
1. **PyPI name and release path** — needs the PMC. PR 1 is deliberately
independent of it.
2. **Which version is the first publishable one?** The tree is
`3.0.0-SNAPSHOT` while `v3.0.0` is tagged, so "wheel version equals engine
version" needs a concrete starting point.
3. **Can you confirm the dist layout** for
`apache-seatunnel-<version>-bin.tar.gz` plus `.sha512` and `.asc` on
`downloads.apache.org`? You state all three exist for 3.0.0; I could not reach
that listing from my network, and PR 1's URL construction depends on it.
On thin vs bundled: agree with thin first. For what it is worth,
`pydebeziumai` (released under the Debezium org last week) also keeps the JVM
engine out of the wheel and fetches the JARs at setup time — downloading the
engine is the established shape for wrapping a JVM engine for pip, not a
compromise.
--
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]
