feat(server): add PATCH /projects/{project_identifier}/retention - #15410
Open
sahildayal wants to merge 3 commits into
Open
feat(server): add PATCH /projects/{project_identifier}/retention#15410sahildayal wants to merge 3 commits into
sahildayal wants to merge 3 commits into
Conversation
Retention policies are standalone, reusable entities that projects point at via Project.trace_retention_policy_id. This endpoint sets which policy a project uses; it never creates, edits, or deletes a policy, so policy CRUD stays with the existing GraphQL mutations. policy_id is required but nullable. An optional field cannot distinguish an omitted key from an explicit null, and one of those means "reset this project's retention" — so requiring the field turns a malformed call into a 422 rather than a silent reset. Null maps to a NULL column, which the schema already defines as "use the default policy". Validation runs before the write: a malformed policy ID is a 422 and an unresolvable one a 404, leaving the existing assignment untouched. Guarded by require_admin and is_not_locked, matching the sibling project routes and the IsAdminIfAuthEnabled on the policy mutations. Closes Arize-ai#12268 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Picks up the js workspace move (Arize-ai#15394), which relocated app/ to js/app/. No source conflicts; the generated TS client change follows the rename.
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
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.
Closes #12268
Summary
Adds
PATCH /v1/projects/{project_identifier}/retention— set which trace-retention policy a project uses, or reset it to the default.Retention policies are standalone, reusable entities (
ProjectTraceRetentionPolicy) that projects point at viaProject.trace_retention_policy_id. This endpoint only changes that pointer: it never creates, edits, or deletes a policy. Policy CRUD stays with the existing GraphQL mutations, per the issue's recommendation to start with reference-by-id only.API shape
policy_idis required but nullable. That is deliberate: with an optional field, "omitted" and "explicitly null" are indistinguishable in Pydantic, and one of those means reset this project's retention. Making it required means a malformed call is a422rather than a silent reset. There's a test pinning that.nullmaps totrace_retention_policy_id = NULL, which the schema already defines as "use the default policy" — so the response echoesnullrather than inventing the default policy's ID.Errors
422— malformedpolicy_id, or the field omitted entirely404— well-formed policy ID that doesn't resolve, or unknown projectPermissions
require_admin+is_not_locked, matching the ADMIN requirement in the issue and the existingupdate_project/delete_projectroutes in this router. The underlying GraphQL mutations carryIsAdminIfAuthEnabledas well.Tests
Nine tests in
TestSetProjectRetentionPolicy(tests/unit/server/api/routers/v1/test_projects.py), all passing on SQLite:nullresets to the default policy404unknown policy, and the project's assignment is unchanged404unknown project422malformedpolicy_id422whenpolicy_idis omittedAlso registered the route in
_ADMIN_ONLY_ENDPOINTSintests/integration/_helpers.py— the coverage assert runs at import time, so a missing entry fails the whole integration suite.I could not run the Postgres matrix locally (no local Postgres/Docker), so that path is covered only by CI here.
Open question from the issue
The issue asks whether this endpoint should also accept an inline policy definition (
{"cron_expression": ..., "rule": {...}}) that creates and attaches a policy in one call. I followed the issue's own recommendation and implemented reference-by-id only. Happy to add inline creation if you'd like it, though it does seem like it belongs with policy CRUD rather than here.Note on overlap
This touches
projects.py, as does #15409 (POST /projects/{id}/clear, for #12267). The two add separate handlers in different parts of the file so they should merge independently, but I'm happy to rebase whichever lands second.