confluentinc / confluentinc/confluent-sql

[refactor] Fold OAuth lifecycle knobs into an `OAuthPolicy` struct + preset globals

Open
#198 0 comments 0 reactions 1 assignee Claimed by @jlrobins View on GitHub
Dominant language
Python
Stars
6
Forks
1
Avg merge
21h 47m
Merged PRs (30d)
26

Description

- **Goal:** replace #156's flat, OAuth-only `reauth` keyword on `connect()`/`Connection` with a
single frozen `OAuthPolicy` dataclass passed as one `oauth_policy` argument, plus a small set of
exported preset instances. This is a pure API-surface refactor — #156's runtime behavior (auto
re-authenticate vs. raise `ReauthenticationRequired`) is unchanged — done **now, while the whole
`auth="oauth"` surface is still unreleased**, so there is no deprecation shim and no migration cost.
It is deliberately landed before #157 so the daemon's opt-in knob arrives as a *field on an
existing struct*, not as yet another `oauth_*` scalar.

- **Why:** `auth="oauth"` already established an `oauth_`-prefixed family of OAuth-only knobs
(`oauth_config`, `oauth_provider_factory`), and #157 is about to add a refresh/daemon toggle.
Left as scalars, that family grows one look-alike parameter per child (`oauth_reauth`,
`oauth_refresh`, …), two of which read near-identically. A struct groups them, lets presets
express *use-cases* instead of the knob cross-product, and — because it is frozen — gives the
#157 one-identity guard a free `__eq__`/`__hash__` to compare policies across connections. This is
the exact shape already used one layer down in this subpackage: `CCloudOAuthConfig` is a
`@dataclass(frozen=True)` and `PROD` is an exported preset instance of it
(`src/confluent_sql/oauth/config.py`). We are applying an existing local pattern, not inventing one.

- **Scope (in):**
- a `@dataclass(frozen=True) OAuthPolicy` in the `oauth` subpackage, exported from
`confluent_sql.oauth` (and re-exported wherever `connect` is imported from, as `PROD` is). One
field for now:
```python
@dataclass(frozen=True)
class OAuthPolicy:
reauth: Literal["auto", "raise"] = "auto"
```
- exported preset instances that name intents rather than knob settings (names provisional):
- `OAUTH_INTERACTIVE = OAuthPolicy(reauth="auto")` — the default; a human is present, so a lapsed
session re-prompts the browser and continues.
- `OAUTH_UNATTENDED = OAuthPolicy(reauth="raise")` — a session that *started* attended but may be
alone at the wall (a long dbt run, an idle notebook kernel); surface
`ReauthenticationRequired` instead of blocking on a browser no one will answer.
- replace `connect()`/`Connection.__init__`'s `reauth` parameter with
`oauth_policy: OAuthPolicy | None = None`. `None` = the default policy (`OAUTH_INTERACTIVE`).
Supersedes the never-shipped `reauth → oauth_reauth` rename idea — there is no point renaming a
scalar we are folding into the struct.
- validation mirrors #156's `reauth` guard in `_resolve_oauth_config`: reject a non-`None`
`oauth_policy` when `auth != "oauth"` ("there is no session lifecycle to govern outside
interactive OAuth"). The per-value check ("reauth must be 'auto' or 'raise'") moves into
`OAuthPolicy` construction / `Literal` typing.
- internal wiring: `Connection` reads `policy.reauth` into its existing per-connection
`_reauth_policy` slot; `_send_with_reauth_policy` is untouched. The struct is a *surface* grouping;
each field still routes to its natural internal home.
- update the #156 tests (`tests/unit/test_connection_oauth_unit.py`) and the
`examples/oauth_data_plane_token_refresh_example.py` comment to the new surface.
- CHANGELOG (Unreleased): note the `reauth=` → `oauth_policy=` surface, still unreleased.

- **Forward-looking (the #157 hook, designed here, implemented there):**
- #157 adds a second field, `refresh: Literal["on-demand", "background"] = "on-demand"`, to
`OAuthPolicy` and a preset such as `OAUTH_LOW_LATENCY = OAuthPolicy(reauth="auto",
refresh="background")`. No new `connect()` parameter; the signature stops growing per child.
- **Cross-connection guard is whole-struct.** The holder adopts "one process = one OAuth identity
**and one policy**": it records the established `OAuthPolicy` beside `self._config`, and its
one-identity guard refuses a second connection whose *explicit* `oauth_policy` differs
(`policy != established`), exactly as it already does for environment and org; an omitted
(`None`) policy inherits and never conflicts. The frozen dataclass is what makes that comparison
a one-liner. This is *stricter* than #156 is today (where `reauth` is silently per-connection
with no guard), but it is unreleased so there is no regression, and multi-connection use is only
ever a factory minting *identical* connections — so the guard is a misconfig backstop that never
fires in practice. Chosen over comparing only the process-wide subset of fields, which would
give the struct's fields inconsistent cross-connection semantics.

- **Out of scope:** the refresh daemon and its park/refcount lifecycle (#157); the `refresh` field
itself (#157 adds it — this ticket only shapes the container so that addition is a one-field
change); any change to #156's re-authentication *behavior*.

- **Depends on:** #156 (revises its surface). **Precedes:** #157 (which extends `OAuthPolicy`).

- **Sized right:** one frozen dataclass + two preset globals + a mechanical kwarg swap and its test
updates, on an unreleased surface — no deprecation path, no behavior change.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.