Skip to content

Add server default result storage config surface#21532

Open
zzstoatzz wants to merge 7 commits intomainfrom
codex/storage-default-config-surface
Open

Add server default result storage config surface#21532
zzstoatzz wants to merge 7 commits intomainfrom
codex/storage-default-config-surface

Conversation

@zzstoatzz
Copy link
Copy Markdown
Collaborator

@zzstoatzz zzstoatzz commented Apr 13, 2026

this PR adds the OSS config surface for a server-level default result storage setting.

What changed
  • adds a ServerDefaultResultStorage schema for representing the configured block document id
  • adds /admin/storage read, update, and clear endpoints
  • adds client methods for reading, updating, and clearing the server default result storage setting
  • validates that the configured block exists and supports write-path
  • keeps storage-default-specific validation and persistence in a dedicated server.models.storage_defaults module instead of leaving that logic inline in the route
  • syncs the generated ui-v2 OpenAPI client for the new admin surface
Why this is a separate slice

This PR only adds the configuration surface for storing a server default result storage block. It does not change runtime result-storage resolution yet.

That separation keeps the persisted setting and API contract reviewable on their own before the follow-up runtime behavior work lands.

This still matches the Cloud backend split: Cloud has a separate storage_defaults model module too, but its propagation and materialization logic lives in later slices. OSS is intentionally stopping at the persisted setting and API contract in this first PR.

Generated client note

ui-v2/src/api/prefect.ts includes the expected /admin/storage additions for this feature.

It also still includes the unrelated IntervalSchedule.interval change from number to number | string. I reproduced that by running npm run service-sync on a clean origin/main worktree, so it appears to be existing generator drift rather than a storage-defaults-specific change.

Verification
  • uv run pyright --ignoreexternal --verifytypes prefect
  • uv run pytest tests/server/api/test_admin.py tests/server/models/test_storage_defaults.py tests/client/test_prefect_client.py -k 'server_default_result_storage'
  • uv run pytest tests/server/models/test_configuration.py tests/server/models/test_storage_defaults.py
  • uv run ruff check src/prefect/server/api/admin.py src/prefect/server/models/__init__.py src/prefect/server/models/configuration.py src/prefect/server/models/storage_defaults.py tests/server/api/test_admin.py tests/server/models/test_configuration.py tests/server/models/test_storage_defaults.py tests/client/test_prefect_client.py
  • repo hooks passed during commit and push, including mypy and Sync UI v2 OpenAPI

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Apr 13, 2026

Merging this PR will not alter performance

✅ 2 untouched benchmarks


Comparing codex/storage-default-config-surface (e7f39ad) with main (1c970c3)

Open in CodSpeed

@zzstoatzz zzstoatzz force-pushed the codex/storage-default-config-surface branch from 6c84fc6 to 18d2756 Compare April 14, 2026 15:03
@zzstoatzz zzstoatzz marked this pull request as ready for review April 14, 2026 15:03
class ServerDefaultResultStorage(PrefectBaseModel):
"""Server-side default result storage configuration."""

default_result_storage_block_id: Optional[UUID] = Field(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this optional? Should it be required since you're also including an endpoint for deletion?

) -> bool:
query = sa.delete(db.Configuration).where(db.Configuration.key == key)
result = await session.execute(query)
db.queries.clear_configuration_value_cache_for_key(key=key)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was looking into this configuration cache, and it looks like it's in memory. We might want to make that configurable because changes to the default storage might not propagate in multi-server environments. That feels like a follow-up PR, though.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants