karthik-0306 commented on issue #11007: URL: https://github.com/apache/seatunnel/issues/11007#issuecomment-5868508839
> [@karthik-0306](https://github.com/karthik-0306) thanks for volunteering. Nobody else in this thread has claimed `connector-paimon`, so it is open for you to take, subject to scope confirmation. > > Before opening a PR, please post a short scope note here so we can confirm it: > > * Which Paimon Source/Sink factory `optionRule()` options are currently marked required on `dev`, and which of those lack a `Conditions.notBlank` (or equivalent declarative) check. > * Confirmation that the change is limited to declarative validation for those existing required options — no new options, no runtime connectivity checks, and no changes to catalog/client construction. > * Planned factory tests covering valid input plus missing, empty, and whitespace-only values for each option you touch. > * Whether the EN/ZH option docs already state these values must be nonblank; update them only if they don't. > > Once the scope is agreed, open one focused PR against `dev` and link it in this thread so the tracker row can be updated. Thanks for confirming. Scope note for connector-paimon: 1. Required options on dev without a declarative blank check (all String, currently plain .required(...)): PaimonSourceFactory: warehouse PaimonSinkFactory: warehouse, database, table PaimonCatalogFactory: warehouse, database, table I'm deliberately leaving catalog_uri (conditional target for catalog_type=hive), source table (in the exclusive with table_list), and all optional options untouched. 2. Limits of the change: only splitting these into .required(OPT, Conditions.notBlank(OPT)). No new options, no change to required/optional status or types, no runtime connectivity checks, and no changes to catalog/client construction or PaimonConfig. Nonblank values with surrounding spaces stay accepted, and I won't add any trimming. 3. Tests: ConfigValidator against each factory's optionRule(): a valid config, plus each touched option missing, empty, and whitespace-only, and a padded nonblank value still accepted. 4. Docs: the EN/ZH source and sink docs mark these options as required but don't say they must be nonblank, so I'll add that to the option descriptions in all four files. One question before I start: in the sink, renameCatalogTable falls back to the incoming table's name/schema when table/database is an empty string. From reading the code, an explicit "" passes today's .required(...) and reaches that fallback, so notBlank on database/table would newly reject it (omitted values are already rejected). Should database and table be in this slice, or should I limit it to warehouse, which already fails at runtime when blank, so runtime behavior stays strictly unchanged? I'll follow your call. -- 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]
