Skip to content

feat(coordination): accept a callable key generator in the KV API - #43435

Merged
villebro merged 1 commit into
gaq-to-gtffrom
villebro/coordination-kv-callable-key
Aug 23, 2026
Merged

feat(coordination): accept a callable key generator in the KV API#43435
villebro merged 1 commit into
gaq-to-gtffrom
villebro/coordination-kv-callable-key

Conversation

@villebro

Copy link
Copy Markdown
Member

SUMMARY

Small, additive ergonomics improvement to the coordination service, part of the GAQ→GTF epic (targets gaq-to-gtf).

CoordinationService.get_value / set_value / delete_value previously took a plain key: str, so every caller had to build the key string eagerly. For parity with Superset's other cache-key helpers (e.g. memoized_func(key=...)), the KV ops now also accept a () -> str key generator, resolved at call time via a shared _resolve_key helper. A literal string still passes through unchanged, so the change is backward-compatible and purely additive; delete_value(*keys) accepts a mix of literals and generators.

KeyLike = Union[str, Callable[[], str]]

A generator that returns a non-string raises TypeError (fail fast, before touching the backend).

Scope note: this is intentionally limited to the general-purpose coordinator KV. The GTF task_key stays a fixed literal — it is the dedup identity resolved synchronously at submit time (it feeds the dedup lock + dedup_key), so it must not become callable/mutable.

TESTING INSTRUCTIONS

  • pytest tests/unit_tests/coordination/test_service.py tests/unit_tests/distributed_lock/ — 27 passed. New coverage: a callable key resolved per-op for get/set/delete (incl. a literal+callable mix), and the non-string-key TypeError guard.
  • mypy / ruff / pylint clean on changed files.
  • Backward compatibility: existing string-key callers (the distributed lock) unchanged and green.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

CoordinationService.get_value / set_value / delete_value took a plain key
string; the caller always built the key eagerly. For parity with Superset's
other cache-key helpers (e.g. memoized_func(key=...)), the KV ops now also
accept a () -> str key *generator*, resolved at call time via a shared
_resolve_key helper (a non-string result raises TypeError). A literal string
still passes through unchanged, so this is backward-compatible and additive.

Note this is scoped to the general-purpose coordinator KV only — the GTF
task_key stays a fixed literal (it is the dedup identity resolved synchronously
at submit time, feeding the dedup lock + dedup_key).
@dosubot dosubot Bot added the api Related to the REST API label Aug 23, 2026
@bito-code-review

bito-code-review Bot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Bito Automatic Review Skipped - Branch Excluded

Bito didn't auto-review because the source or target branch is excluded from automatic reviews.
No action is needed if you didn't intend for the agent to review it. Otherwise, to manually trigger a review, type /review in a comment and save.
You can change the branch exclusion settings here, or contact your Bito workspace admin at evan@preset.io.

@github-actions github-actions Bot removed the api Related to the REST API label Aug 23, 2026
@netlify

netlify Bot commented Aug 23, 2026

Copy link
Copy Markdown

Deploy Preview for superset-docs-preview ready!

Name Link
🔨 Latest commit c6cc291
🔍 Latest deploy log https://app.netlify.com/projects/superset-docs-preview/deploys/6a8b348fdad2ae00082f6e94
😎 Deploy Preview https://deploy-preview-43435--superset-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@villebro
villebro merged commit 8489b0b into gaq-to-gtf Aug 23, 2026
58 checks passed
@villebro
villebro deleted the villebro/coordination-kv-callable-key branch August 23, 2026 18:04
@codecov

codecov Bot commented Aug 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.84%. Comparing base (5719f04) to head (c6cc291).
⚠️ Report is 2 commits behind head on gaq-to-gtf.

Additional details and impacted files
@@             Coverage Diff             @@
##           gaq-to-gtf   #43435   +/-   ##
===========================================
  Coverage       78.84%   78.84%           
===========================================
  Files            2880     2880           
  Lines          164425   164459   +34     
  Branches        37995    38003    +8     
===========================================
+ Hits           129636   129673   +37     
+ Misses          32346    32341    -5     
- Partials         2443     2445    +2     
Flag Coverage Δ
hive 38.13% <45.45%> (-0.02%) ⬇️
mysql 57.68% <45.45%> (-0.03%) ⬇️
postgres 57.71% <45.45%> (-0.03%) ⬇️
presto 40.07% <45.45%> (-0.02%) ⬇️
python 83.53% <100.00%> (+0.01%) ⬆️
sqlite 57.41% <45.45%> (-0.03%) ⬇️
unit 73.62% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant