shangeyao commented on PR #4436:
URL: https://github.com/apache/streampark/pull/4436#issuecomment-5019742358
Thanks for picking up #4408 Phase 0.1 — the scoped approach and lifecycle
tests look good.
Before merge, I think we need closer parity with the old `FlinkStreaming`
platform contract:
1. **Init path** — Wire init through `FlinkStreamingInitializer` (or an
equivalent Java port): `--conf`, `setAppHome`, merged `ParameterTool` /
`FlinkConfiguration`. The current `ParameterTool.fromArgs()` + bare
`StreamExecutionEnvironment.getExecutionEnvironment()` won't work for
Console-submitted jobs.
2. **App name** — Use StreamPark app-name resolution (`getAppName`
semantics: `app.name`, `pipeline.name`, deflate), not only raw
`parameter.get("app.name", ...)`. The existing test doesn't verify the name
passed to `execute()`.
3. **`main()` entry** — Console launches user `mainClass`; the old trait
provided `main()`. Please document or provide a standard entry pattern for
`FlinkStreamingJob`.
4. **Context vs #4432** — Clarify in the PR description that this
reintroduces `streampark-flink-core` as the **Java replacement** after #4432,
per #4408 — not a rollback.
Non-blocking: consider `finally` for `destroy()`, align package name with
#4408 (`core.java`?), add `streampark-common` (provided), and update
`AGENTS.md` when this lands.
Happy to re-review once the blocking items are addressed. +1 on phased
follow-ups for `FlinkTable` / `FlinkStreamTable`.
--
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]
