[WIP] feat: add support for write conflict settings #224
Conversation
changes: - No changes to the client interface - API changes include support for write conflict options API commit: 0ac19aac54f21f3c78970126b84b4c69c6e3b9a2
WalkthroughIntroduces idempotency controls for Write requests via new on_duplicate and on_missing fields in models and documentation. Updates API docstrings to describe semantics and examples. Adjusts OAuth2 token URL derivation logic. Makes minor formatting changes in REST client and tests. Removes two entries from .openapi-generator/FILES. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Client App
participant SDK as OpenFGA Python SDK
participant S as OpenFGA Server
Dev->>SDK: write({ writes: { tuple_keys, on_duplicate },<br/>deletes: { tuple_keys, on_missing } })
note right of SDK: Prepare request payload
SDK->>S: POST /write (payload)
rect rgba(200,220,255,0.25)
note over S: Server evaluates per-subrequest<br/>• on_duplicate: error|ignore<br/>• on_missing: error|ignore<br/>Precedence: any error → fail
end
alt All sub-operations allowed
S-->>SDK: 200 OK
SDK-->>Dev: Success
else A conditioned sub-request flagged "error" fails
S-->>SDK: 4xx Error
note over S: Transaction rolled back
SDK-->>Dev: Error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Pre-merge checks✅ Passed checks (3 passed)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (11)
openfga_sdk/sync/oauth2.py (1)
73-73: Use of private helper_parse_issuer—consider stable public surface.Delegating token URL derivation here is good. However, calling a private method (
_parse_issuer) couples OAuth2Client to Credentials internals. Prefer promoting it to a public helper (e.g.,parse_issuer) or module-level utility to avoid future breakage.Please confirm these issuer inputs behave as intended with the new parser:
- "auth.example.com" → "https://auth.example.com/oauth/token"
- "https://auth.example.com" → "https://auth.example.com/oauth/token"
- "https://auth.example.com/custom/token" → unchanged
- "http://auth.example.com" (http allowed) → "http://auth.example.com/oauth/token"
docs/OpenFgaApi.md (1)
1294-1294: Fix markdownlint MD038 (no-space-in-code).The updated Write section triggers MD038: “Spaces inside code span elements.” Remove any leading/trailing spaces inside inline code spans on this line (e.g., backticked snippets like
writes,deletes, JSON fragments).Apply a minimal cleanup such as trimming spaces inside backticks in this paragraph to satisfy markdownlint.
docs/WriteRequestWrites.md (1)
8-8: Clarify allowed values and reference.Consider explicitly listing allowed values as
{ "error", "ignore" }and linking “RelationshipCondition” to its doc to make the equality criteria unambiguous.-**on_duplicate** | **str** | On 'error' ( or unspecified ), the API returns an error if an identical tuple already exists. On 'ignore', identical writes are treated as no-ops (matching on user, relation, object, and RelationshipCondition). | [optional] [default to 'error'] +**on_duplicate** | **str** | One of `error` (default) or `ignore`. On `error`, the API returns an error if an identical tuple already exists. On `ignore`, identical writes are treated as no-ops (matching on user, relation, object, and [RelationshipCondition](RelationshipCondition.md)). | [optional] [default to `error`]docs/WriteRequestDeletes.md (1)
8-8: Mirror value set and tighten wording.Align wording with writes and make the options explicit to reduce ambiguity.
-**on_missing** | **str** | On 'error', the API returns an error when deleting a tuple that does not exist. On 'ignore', deletes of non-existent tuples are treated as no-ops. | [optional] [default to 'error'] +**on_missing** | **str** | One of `error` (default) or `ignore`. On `error`, deleting a non-existent tuple returns an error. On `ignore`, such deletes are treated as no-ops. | [optional] [default to `error`]openfga_sdk/sync/open_fga_api.py (2)
2693-2714: Mirror the same docstring nits in write_with_http_info.Replicate the grammar/spacing tweaks and optional default clarifications here to keep both method docstrings consistent.
2666-2686: Docstring: fix grammar, tighten parentheses, and confirm user-facing term "condition"Verified: use "condition" in user-facing docstrings (RelationshipCondition is the internal protobuf type).
Apply this minimal diff:
- The Write API will transactionally update the tuples for a certain store. Tuples and type definitions allow OpenFGA to determine whether a relationship exists between an object and an user. + The Write API will transactionally update the tuples for a certain store. Tuples and type definitions allow OpenFGA to determine whether a relationship exists between an object and a user. - On 'error' ( or unspecified ), the API returns an error if an identical tuple already exists. + On 'error' (or unspecified), the API returns an error if an identical tuple already exists. - On 'ignore', deletes of non-existent tuples are treated as no-ops. + On 'ignore', deletes of non-existent tuples are treated as no-ops.
- Optional: explicitly state defaults for on_duplicate/on_missing if you can verify them in the API contract.
openfga_sdk/models/write_request_deletes.py (2)
44-59: Constructor default is sensible; consider documenting it inline.Defaulting on_missing to "error" aligns with the API semantics. Consider adding “Default: 'error'.” to the property docstring for quick discoverability.
- """Gets the on_missing of this WriteRequestDeletes. + """Gets the on_missing of this WriteRequestDeletes. (Default: "error")
83-115: Enum validation LGTM; optional tiny allocation nit.Validation against ["error", "ignore"] is correct. If we want to avoid re-allocating the list on each setter call, hoist it to a module/class-level constant. Not required.
+ON_MISSING_ALLOWED = ("error", "ignore") ... - allowed_values = ["error", "ignore"] - if ( - self.local_vars_configuration.client_side_validation - and on_missing not in allowed_values - ): + if ( + self.local_vars_configuration.client_side_validation + and on_missing not in ON_MISSING_ALLOWED + ): raise ValueError( - "Invalid value for `on_missing` ({0}), must be one of {1}".format( - on_missing, allowed_values + "Invalid value for `on_missing` ({0}), must be one of {1}".format( + on_missing, ON_MISSING_ALLOWED ) )openfga_sdk/models/write_request_writes.py (3)
44-59: Constructor default is sensible; consider documenting it inline.Defaulting on_duplicate to "error" aligns with non-idempotent-by-default semantics. Consider noting the default in the property docstring.
- """Gets the on_duplicate of this WriteRequestWrites. + """Gets the on_duplicate of this WriteRequestWrites. (Default: "error")
83-115: Docstring micro-nit and optional constant for allowed values.
- “( or unspecified )” → “(or unspecified)”.
- Optional: hoist allowed values to a constant to avoid reallocation.
- On 'error' ( or unspecified ), the API returns an error if an identical tuple already exists. + On 'error' (or unspecified), the API returns an error if an identical tuple already exists.+ON_DUPLICATE_ALLOWED = ("error", "ignore") ... - allowed_values = ["error", "ignore"] - if ( - self.local_vars_configuration.client_side_validation - and on_duplicate not in allowed_values - ): + if ( + self.local_vars_configuration.client_side_validation + and on_duplicate not in ON_DUPLICATE_ALLOWED + ): raise ValueError( - "Invalid value for `on_duplicate` ({0}), must be one of {1}".format( - on_duplicate, allowed_values + "Invalid value for `on_duplicate` ({0}), must be one of {1}".format( + on_duplicate, ON_DUPLICATE_ALLOWED ) )
87-100: Use JSON field name "condition" in the docstring and note SDK type RelationshipConditionOpenFGA Write API examples use the JSON field "condition"; update the docstring to say "matching on user, relation, object, and condition (SDK type: RelationshipCondition)" so it matches API examples while preserving the SDK type name.
File: openfga_sdk/models/write_request_writes.py (lines 87–100)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.openapi-generator/FILES(0 hunks)docs/OpenFgaApi.md(1 hunks)docs/WriteRequestDeletes.md(1 hunks)docs/WriteRequestWrites.md(1 hunks)openfga_sdk/api/open_fga_api.py(2 hunks)openfga_sdk/models/write_request_deletes.py(2 hunks)openfga_sdk/models/write_request_writes.py(2 hunks)openfga_sdk/sync/oauth2.py(1 hunks)openfga_sdk/sync/open_fga_api.py(2 hunks)openfga_sdk/sync/rest.py(1 hunks)test/sync/rest_test.py(8 hunks)
💤 Files with no reviewable changes (1)
- .openapi-generator/FILES
🧰 Additional context used
🧬 Code graph analysis (3)
openfga_sdk/sync/oauth2.py (1)
openfga_sdk/credentials.py (5)
_parse_issuer(180-205)configuration(167-171)configuration(174-178)api_issuer(95-99)api_issuer(102-106)
openfga_sdk/models/write_request_deletes.py (1)
openfga_sdk/configuration.py (2)
Configuration(120-742)get_default_copy(355-366)
openfga_sdk/models/write_request_writes.py (3)
openfga_sdk/models/write_request_deletes.py (2)
tuple_keys(61-68)tuple_keys(71-81)openfga_sdk/models/contextual_tuple_keys.py (2)
tuple_keys(50-57)tuple_keys(60-70)openfga_sdk/configuration.py (2)
Configuration(120-742)get_default_copy(355-366)
🪛 markdownlint-cli2 (0.17.2)
docs/OpenFgaApi.md
1294-1294: Spaces inside code span elements
(MD038, no-space-in-code)
1294-1294: Spaces inside code span elements
(MD038, no-space-in-code)
🔇 Additional comments (6)
openfga_sdk/sync/rest.py (1)
158-158: Whitespace-only tweak looks good.The blank line after the init docstring improves readability without behavior changes.
test/sync/rest_test.py (1)
538-539: LGTM on decorator and style updates.Quoting and assertion style changes are consistent and do not alter behavior.
Also applies to: 565-566, 597-601, 630-631, 665-669, 704-705, 732-739, 744-746
openfga_sdk/api/open_fga_api.py (1)
2670-2671: Docstring updates for Write semantics—looks good.The SDK docs now accurately reflect
on_duplicate/on_missingsemantics and precedence; no API surface changes.Also applies to: 2697-2698
openfga_sdk/models/write_request_deletes.py (2)
34-42: Mapping looks correct.openapi_types and attribute_map correctly introduce on_missing and will serialize as expected.
70-81: Verify OpenAPI schema: ensure minItems for WriteRequestDeletes.tuple_keys if non-empty lists are requiredSetter in openfga_sdk/models/write_request_deletes.py (lines 70–81) uses the generator-pattern None-only check. If the API intends to forbid empty lists, add minItems: 1 to WriteRequestDeletes.tuple_keys in the OpenAPI schema so client-side validation rejects []. Repository search returned no vendored OpenAPI schema — confirm schema location/path or update the upstream spec.
openfga_sdk/models/write_request_writes.py (1)
34-42: Mapping looks correct.openapi_types/attribute_map correctly add on_duplicate with expected shape.
rhamzeh
changed the title
rhamzeh
changed the title
changes:
API commit: 0ac19aac54f21f3c78970126b84b4c69c6e3b9a2
Description
What problem is being solved?
How is it being solved?
What changes are made to solve it?
References
Review Checklist
mainSummary by CodeRabbit
New Features
Documentation
Chores