apache / apache/iggy

Add sink/source connector templates under core/connectors/ as fill-in-the-blank starting points

Open
#3,956 0 comments 1 reaction 1 assignee Claimed by @ryerraguntla View on GitHub
connectors docs enhancement
Dominant language
Rust
Stars
4.9k
Forks
432
Avg merge
2d 10h
Merged PRs (30d)
173

Description

### 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

Connector new Development

### 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

- [x] 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

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.