ryerraguntla opened a new issue, #3956:
URL: https://github.com/apache/iggy/issues/3956
### Description
## Motivation
Connector PRs repeatedly lose review cycles to the same non-backend-specific
issues: secrets typed as plain `String`, missing ACK/NACK cursor staging
(#3855), transient/permanent error mis-mapping, config knob name drift,
missing canonical state tests. `.claude/skills/connector-pr-review/SKILL.md`
and `core/connectors/docs/authoring-sinks-and-sources.md` already codify
these as a checklist, but authors still hand-roll the plumbing each time.
Proposal: ship two ready-to-copy template crates — `sink_template` and
`source_template` — under `core/connectors/sinks/` and
`core/connectors/sources/`,
already implementing every framework-level requirement, with the
backend-specific gaps marked `TODO(Developer)`.
### Affected area / component
_No response_
### Proposed solution
## Functionality covered implemented in the templates
- Config parsing with `#[serde(deny_unknown_fields)]`.
- Structural + connectivity validation in `open()` (not `new()`).
- `SecretString` on every credential-shaped field (`connection_string`,
`auth_token`), via `iggy_common::serde_secret::serialize_secret`.
- Retry-wrapped client (`iggy_connector_sdk::retry::build_retry_client`) +
startup connectivity probe (`check_connectivity_with_retry`).
- `CircuitBreaker` consulted before each call, recorded once per
`consume()`/`poll()` (not per-chunk).
- Sink: batching by configurable `batch_size`, `last_err` pattern (never
swallows a failed batch into `Ok(())`), identifier validation before
interpolation into any query/path/URL.
- Source: full PR #3855 ACK/NACK contract — `poll()` stages candidate state
in `pending_state`, `on_batch_result()` commits on `Ack` / discards on
`Nack`; sleep-first poll; circuit-breaker-open returns empty result
instead of `Err` (avoids busy-loop retry with no delay).
- Canonical test suites: sink 6 tests, source 8 tests (4 state: restore /
no-state / invalid-state / round-trip, + 2 ACK/NACK, + connection-string
validation + circuit-breaker short-circuit).
- Apache license headers, `crate-type = ["cdylib", "lib"]`, workspace-pinned
deps, `cargo-machete` ignore for macro-only deps (`dashmap`).
## What a plugin author still has to write
Marked `TODO(Developer)` in `src/lib.rs`:
- Sink: one spot — `push_batch()`.
- Source: two spots — `build_raw_client()` (if not HTTP) and
`fetch_records()`.
## Verification done so far (local, not yet in-tree)
Checked out against `apache/master` (post-#3855) in an isolated git
worktree:
- `cargo check -p iggy_connector_template_sink -p
iggy_connector_template_source --all-targets` — clean.
- `cargo clippy --all-targets -- -D warnings` — clean.
- `cargo test` — 6/6 (sink) + 8/8 (source) passing.
Fixed during review: `connection_string` was plain `String` in both
templates (should always be `SecretString` per
`connector-pr-review/SKILL.md` blocker B1); `source_template` was missing
the round-trip state test.
## Proposed scope for this issue
1. Move `sink_template/` → `core/connectors/sinks/sink_template/`,
`source_template/` → `core/connectors/sources/source_template/`.
2. Add both to workspace `members` in root `Cargo.toml`.
3. Add rows to `core/connectors/sinks/README.md` / `sources/README.md`.
4. Cross-link from
`core/connectors/docs/authoring-sinks-and-sources.md` and the
`connector-sink` / `connector-source` skill `TEMPLATE.md` files (which
currently only describe the pattern in prose — could point at compiling
code instead, or be generated from it).
5. Decide: do the templates build as real (inert) plugins in CI, or should
they be excluded from the default release build (`publish = false` is
already set)?
## Open questions
- Should `TEMPLATE.md` fill-in-the-blank kits in the skills be replaced by
"copy this crate" instructions, kept as a lighter-weight alternative, or
generated from the template source so they can't drift?
- Naming: `sink_template` / `source_template` vs. something like
`_template_sink` to sort away from real plugins in directory listings?
## Non-goals for this issue
Not proposing to change any SDK trait surface — templates target the current
`Sink`/`Source` traits as-is (including the #3855 ACK/NACK contract already
merged to master).
### Alternatives considered
_No response_
### Contribution
- [ ] I'm willing to submit a pull request to implement this feature
### Good first issue
- [ ] I think this could be a good first issue for a new contributor
--
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]
