erica1111-prog commented on issue #11007: URL: https://github.com/apache/seatunnel/issues/11007#issuecomment-6679798034
> [@erica1111-prog](https://github.com/erica1111-prog) — thanks for posting the scope note, that covers most of what was asked. A few things before this slice is confirmed here: > > 1. **Commit inspected**: "current latest `dev`" isn't trackable — please post the exact `dev` commit SHA you inspected so the slice can be pinned the same way as the others. > 2. **Mutual-exclusion rule**: the description reads as "`KEY_MEASUREMENT_RULE` bundled with optional `KEY_MEASUREMENT_FIELD` vs. standalone `KEY_MEASUREMENT_RULE`", which has `KEY_MEASUREMENT_RULE` on both sides. Could you restate precisely which options are in each exclusive group as you intend to encode them in `optionRule()`? > 3. **Test coverage**: `testSinkOptionRulesSuccess`, `testSinkOptionRuleMissingRequired` and `testSinkOptionRuleMutualExclusion` look good. The earlier ask also mentioned blank / padded values for the required options (`URL`, `DATABASES`) — please say whether you plan to cover those in `InfluxDBFactoryTest` or whether you're deliberately keeping them out of this slice (either is fine, just make it explicit). > 4. **Docs**: agreed that if EN/ZH already list these options with matching required/optional status, no contract change is needed — please just double-check the required/optional markers match what `optionRule()` will enforce. > 5. Your comment appears to be cut off at "zero runti" — please repost the last part so the full note is on record. > > Once those are in, the `connector-influxdb` sink slice can be confirmed here (no assignment change yet). Please keep the change limited to `InfluxDBSinkFactory` and `InfluxDBFactoryTest`, as you described. @SEZ9 Thanks for the detailed feedback! Here are the clarifications: 1. **Exact dev commit SHA**: Inspected base commit: `c875a87bd6fb238585dd6856c2c4233b6a8ece46` 2. **Precise Mutual-Exclusion Rule**: Apologies for the typo in the previous note. The mutually exclusive groups encoded in `optionRule()` are: - **Group A (Dynamic measurement)**: bundled `KEY_MEASUREMENT_RULE` with optional `KEY_MEASUREMENT_FIELD` - **Group B (Static measurement)**: standalone `MEASUREMENT` A sink configuration cannot supply both static `MEASUREMENT` and dynamic rule-based measurement simultaneously. 3. **Blank / Padded Values Scope**: To keep this slice strictly isolated to structural `OptionRule` migration without introducing changes to underlying runtime URL/database connection parsing, blank/whitespace-only/padded string validation checks are deliberately kept out of this slice. The test coverage focuses strictly on: - Valid configurations (happy path with required & optional options). - Missing required options (`URL`, `DATABASES`). - Mutual-exclusion / bundling conflicts between `MEASUREMENT` and `KEY_MEASUREMENT_RULE`. 4. **Documentation Contract**: Double-checked both `docs/en/connector-v2/sink/InfluxDB.md` and `docs/zh/connector-v2/sink/InfluxDB.md`: `url` and `database` are already marked as required (`yes`), while `measurement`, `key_measurement_rule`, and optional connection/batching params are marked as optional (`no`). The contract strictly matches the rules enforced by `optionRule()`, so no doc updates are needed. 5. **Complete Closing Note**: "All changes are strictly isolated to `InfluxDBSinkFactory` and `InfluxDBFactoryTest` with zero runtime changes to the InfluxDB sink execution logic." -- 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]
