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]

Reply via email to