SK-3118: FlowDB support for Python SDK (skyflow-flowvault) + module segregation - #276
Open
saileshwar-skyflow wants to merge 19 commits into
Open
SK-3118: FlowDB support for Python SDK (skyflow-flowvault) + module segregation#276saileshwar-skyflow wants to merge 19 commits into
saileshwar-skyflow wants to merge 19 commits into
Conversation
…lowdb (v3) insert support Restructures the repo into three build variants sharing a bundled common/ module (SK-2938 Option C): v2 (today's SDK, behavior-preserving) and a new v3 built on the flowservice/flowdb API, insert-only this round. common/ - Shared credential resolution, vault-URL resolution, and bearer-token fetch/cache/expiry logic (VaultController, BaseVaultClient), enums, errors, service_account, and generic validators. - VaultController declares insert/get/update/delete/query/detokenize as abstract methods (Java-interface-style); v2 and v3 each provide their own concrete/stub implementations. v2 - Relocated from the repo root via git mv; public API unchanged (same class names, signatures, import paths). Vault is now a backward-compatible alias for the internal PdbVaultController class. - Fixed a latent bug where v2's own Env enum failed cross-class comparisons against common's Env; both now share one definition. v3 (skyflow-flowvault, starting at 1.0.0) - New InsertRequest/InsertRecord/Upsert/InsertResponse types, FlowVaultController, and VaultClient targeting the flowservice REST API. - Insert validation ported from Java's v3 Validations.java: table/upsert must live in exactly one place (request-level or per-record, matching in both), 10k record cap, empty key/value checks. - InsertResponse mirrors Java's v3 shape (summary/success/errors) as plain dicts, each result tagged with its index in the original record list (stable across batch boundaries). - Structured per-record error parsing from the backend's actual error body, plus x-request-id propagation onto error entries. - Batching via INSERT_BATCH_SIZE (default 50, max 1000), sequential, isolate-and-continue on a failing batch. - Vault URL resolution uses v3's own skyvault.skyflowapis.* domain for all four envs (DEV/SANDBOX/STAGE/PROD), confirmed to differ from v2's vault.skyflowapis.* domain. CI/CD - shared-tests.yml and shared-build-and-deploy.yml now take a `variant` input and scope every step to v2/ or v3/ via working-directory. - main.yml, ci.yml, beta-release.yml, internal-release.yml, and release.yml matrix over both variants. v3 releases are distinguished from v2's via a flowvault- tag/branch prefix (flowvault-1.0.0, flowvault-release/*) so a release trigger is never ambiguous between the two independently-versioned packages; v2's existing bare-semver tags are untouched. - Fixed ruff.toml/.codespellrc still excluding a pre-split "skyflow/generated" path that no longer existed after the relocation. - Fixed a bump_version.sh sed collision with a comment that happened to contain the literal text "__version__ = ...". - Added a common/ test job to main.yml/ci.yml. Note: v3/samples/ and the root samples/ folder are deliberately excluded from this branch/commit -- local working copies there contain credentials used for live testing against a real vault and must not be pushed. Tests: common 36, v3 65, v2 426 (2 pre-existing unrelated fixture failures), tests/contract passing for both variants. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Distribution name was already skyflow-flowvault (setup.py) but the importable package stayed skyflow, colliding with v2's skyflow import name if both are ever installed in the same environment. Rename the v3 directory to flowvault and its package to skyflow_flowvault to match the distribution name and remove the collision. generated/ content is left untouched (Fern-owned).
…d base insert response Consolidates duplicated logic between v2 (PDB) and flowvault per architecture review: shared validation (vault config, credentials, log level), LogLevel/Logger, and insert field/table validation now live in common with per-variant message injection; adds BaseInsertResponse alongside BaseInsertRequest so each variant's InsertRequest/InsertResponse can extend a common base while keeping its own shape. Also fixes flowvault's insert() response shape (drop redundant 'data'/'table', flatten tokens, errors=None when empty) and a stale SDK_VERSION drift bug. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…insert() Splits BaseSkyflow, BaseVaultController, and BaseVaultClient into a pure interface (ISkyflow/IVaultController/IVaultClient, declaring the contract via ABC + abstractmethod) plus a base class implementing the shared logic, so future variant-specific overrides have a clear contract to satisfy. Also adds type hints to insert() at every layer (BaseInsertRequest/BaseInsertResponse in the abstract method, each variant's own InsertRequest/InsertResponse in their concrete override), and renames base_vault.py to base_vault_controller.py to match its class name. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…shape, add types BaseInsertRequest's shared field is now named values (matching PDB's terminology) instead of records, and table/values are required rather than defaulted; each variant's InsertRequest forwards them explicitly. Removes flowvault's Upsert class in favor of a plain dict (now typed via a TypedDict) to match the rest of flowvault's dict-based request shape, and adds return type hints to every method on v2's VaultController to match insert's. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
BaseSkyflow implements every ISkyflow abstract method, so ABC alone doesn't block instantiating it directly -- only make_skyflow_class-produced subclasses should be constructed. Raises SkyflowError with a new SkyflowMessages entry instead of a raw NotImplementedError, matching how every other SDK error is raised. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
CI workflows (ci.yml, main.yml, release.yml, beta-release.yml, internal-release.yml) still referenced the pre-rename v3/ directory, so every flowvault CI job failed outright. Threads a package-name (skyflow vs skyflow_flowvault) through shared-tests.yml/shared-build-and-deploy.yml/ bump_version.sh, since those hardcoded the skyflow package name too -- a real release would've bumped the wrong version file. Also fixes common/setup.py's missing python-dotenv dependency (test-common CI job installs only this file's declared deps), and removes the empty-value insert tests now that empty/null field values are explicitly allowed rather than rejected. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… noise
flowvault/requirements.txt was missing coverage, so the v3 test job's
`python -m coverage run` step failed outright with "No module named
coverage" once the workflow correctly pointed at flowvault. Also excludes
**/generated/** (Fern-owned) from semgrep, since the generated REST clients
trip its secret-detection heuristics on parameter names like `token`, and
fixes a real semgrep finding: shared-build-and-deploy.yml interpolated
${{ }} context values directly into a run: shell block instead of routing
them through env: first.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…e positives test-common never ran coverage or uploaded to Codecov, so common/ (which grew substantially this session) was invisible to Codecov's patch/project checks. Adds a coverage run + Codecov upload step matching v2/flowvault's pattern. Also fixes .semgreprules/customRule.yml's check-sensitive-info regex: an optional quote-capture group let its own backreference match empty string, so any `keyword: value` matched regardless of quoting -- tightened to require an actual quoted literal and exclude self-referential values (e.g. TOKEN = 'token'), which eliminates 28 false positives without any inline suppressions. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…tion/detect structurally absent when unsupported Renames ISkyflow -> BaseSkyflow (pure interface) and the old BaseSkyflow -> BaseSkyflowImpl (concrete). Moves connection/detect support into ConnectionCapable/DetectCapable interfaces + ConnectionMixin/DetectMixin in a new common/client/utils/_utils.py, conditionally composed into a variant's Skyflow class by make_skyflow_class() so unsupported variants (e.g. flowvault) genuinely lack .connection()/.detect() (AttributeError) instead of raising NotImplementedError from a present-but-guarded method. Also fixes two review-flagged bugs: adding a vault/connection config with a duplicate id to an already-built client now raises SkyflowError instead of silently overwriting the existing entry, and update_connection_config no longer risks a bare KeyError on a missing connection_id. Extracts the Builder's raw NotImplementedError string literals into named constants. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… common Main advanced with the 2.1.3 release and SK-3039 (read the token roles/context from credentials, not the vault config, and validate them). Those changes landed on the old root-level `skyflow/` tree, which this branch has split into `common/` + `skyvault/`, so they are re-applied where that logic now lives: - common/utils/validations: add validate_token_options (roles/context checks, broadened context types, role-element validation) and call it from validate_credentials. - common/vault/base_vault_client.get_bearer_token: validate token options and build role_ids/ctx from credentials instead of config. - common/service_account._validate_and_resolve_ctx: take a messages param so delegated skyvault errors keep skyvault's SDK version in their text. - skyvault validations delegate validate_token_options to common; skyvault bumped to 2.1.3 (setup.py + _version.py). - SK-3039 client tests ported to common/tests/vault/test_base_vault_client.py (patch targets -> common.vault.base_vault_client) and the skyvault end-to-end tests repointed accordingly. - shared-build-and-deploy.yml: keep module `ref` input alongside main's SK-2986 admin-token comment. - flowvault bulk response: extract retryable-HTTP-code literals into constants (clears ruff PLR2004 that was failing CI). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Semgrep OSS found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
The shared per-module test loop aborted on the first module (common) and `bash -e` then skipped skyvault and flowvault, so their tests never ran on a PR. Two fixes: - Add the repo root to PYTHONPATH for the test run. common/ is imported as a namespace package (common.vault has no __init__), so its own wheel can't be imported as `common`; resolving from the source tree fixes discovery for all modules. - Run every module even if one fails and fail the job only at the end, so one module never hides the others' results. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The Fern-generated REST clients are not hand-written and were dragging module
coverage down (failing Codecov) and producing 82 of 83 Semgrep code-scanning
alerts.
- codecov.yml: ignore **/generated/**.
- Add .coveragerc omit for */generated/* to common, skyvault and flowvault so
generated code is not measured (flowvault 96.6%, common 86.6% after).
- semgrep.yml: pass --exclude generated so the SARIF upload no longer flags
generated code.
- shared-build-and-deploy.yml: use the built-in $GITHUB_ACTOR env var instead
of interpolating ${{ github.actor }} in a run step (the remaining Semgrep
shell-injection finding).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…v targets
- shared-build-and-deploy.yml: route inputs/steps-outputs/github.ref_name
through an env: block so no untrusted ${{ }} is interpolated in the run:
script (clears the last Semgrep run-shell-injection alert).
- codecov.yml: the base's auto-target is 99.75% (mature v2 code), which a large
PR adding new flowvault/common code can't hit; allow a 5% project threshold
and an 85% patch target so codecov reflects real, healthy coverage
(project 95.4%, patch 90.6%) instead of blocking on the inherited target.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
saileshwar-skyflow
force-pushed
the
saileshwar/SK-3118-flowdb-support-python-sdk
branch
from
September 1, 2026 17:28
4f7502d to
8ef7674
Compare
Port the Java SDK's japicmp contract gate to Python using griffe (static API analysis), for both published packages: - ci-scripts/contract/griffe_contract.py: builds each package's public-API surface from an explicit module allowlist (mirrors Java's <includes>; excludes generated/ and internal utils helpers) and dumps/checks it against a committed baseline. Removed/changed entries are breaking, added entries are new surface; any drift fails. - Committed baselines skyvault/api-report/skyflow.api.json (358 members) and flowvault/api-report/skyflow_flowvault.api.json (153). - ci-scripts/contract-snapshot-update.sh: regenerate baselines after an intentional public API change. - .github/workflows/contract-tests.yml: per-module matrix gate (fail-fast false), a skyvault-only guard `griffe check skyflow -a skyflow==2.1.3` (no breaking changes vs the released skyflow), and a PR comment showing the baseline diff when it changes. - Add griffe[pypi] to each module's dev extras. Verified skyvault's public surface has no breaking changes vs released skyflow 2.1.3. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Public API contract change (
|
Public API contract change (
|
griffe 2.2.0 requires Python >= 3.10, so `pip install griffe==2.2.0` failed on the 3.9 runner. griffe analyses the SDK source statically, so the analyzer's Python version is independent of the SDK's own >= 3.9 support and can be 3.10. Also mark the `griffe` dev extra `python_version >= "3.10"` so a 3.9 dev install does not fail. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Java's contract test compares only against the committed baseline, never a published release, so remove the skyvault-only `griffe check -a skyflow==2.1.3` step and the SKYVAULT_RELEASE bump it needed. The committed baseline is the contract for both modules. Current skyvault was verified to have no breaking changes vs released skyflow 2.1.3. Drop the now-unneeded griffe pypi extra. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds FlowDB (Flow DB vault) support to the Python SDK as a new published package,
skyflow-flowvault, and segregates the repo into independently-built modules.Modules
common/— shared insert/validation/logging/auth logic.skyvault/— the existing Privacy DB SDK (published asskyflow), renamed fromv2/.flowvault/— new Flow DB SDK (published asskyflow-flowvault), built on the regenerated Fern REST client.flowvault operations
recordslist with normalized tokens / hashed_data / metadata.bulk_insert/bulk_detokenize(sync + async) with env-var-driven batching and concurrency, order-preserving indexing, andrecords_to_retry()/tokens_to_retry()helpers.InsertRequestRecord,UpsertOptions,ColumnRedaction, bulk request/response/summary types).Docs & samples (Java-style layout)
README.mdrouter → per-module comprehensive READMEs (flowvault/README.md,skyvault/README.md).setup.pyreads its own module README for the PyPIlong_description.samples/trees (samples excluded from wheel + sdist viafind_packages(exclude=[...])andMANIFEST.in prune samples).flowvault/CONTRACT_SHAPES.mdrequest/response JSON reference.Build / CI
ci-scripts(bump_version.sh,current_module_version.sh)..github/workflowsupdated for per-module builds (addspr.yml,pr-flowvault.yml; retiresbeta-release.yml,ci.yml).Testing
long_descriptionresolves to its own README.🤖 Generated with Claude Code