-
Notifications
You must be signed in to change notification settings - Fork 467
feat(api-docs): Generate OpenAPI 3.1 docs with drf-spectacular #6451
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
Docker builds report
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6451 +/- ##
==========================================
+ Coverage 98.07% 98.08% +0.01%
==========================================
Files 1294 1296 +2
Lines 46537 46616 +79
==========================================
+ Hits 45639 45724 +85
+ Misses 898 892 -6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@Zaimwa9 Yeah, looks like the typing and test coverage sections went over Claude's head here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
flagsmith/api/edge_api/identities/serializers.py
Lines 167 to 168 in 33ba2ee
| class Meta: | |
| swagger_schema_fields = {"type": "integer/string"} |
I believe this type hack needs to be turned into a anyOf too
I started on it the other day and ended up using OpenApiSerializerFieldExtension to map these custom fields to proper anyOf schema.
Like this if it can help:
class EdgeFeatureFieldExtension(OpenApiSerializerFieldExtension):
target_class = "edge_api.identities.serializers.EdgeFeatureField"
def map_serializer_field(self, auto_schema, direction):
if direction == "request":
return {
"anyOf": [
{"type": "integer", "description": "Feature ID"},
{"type": "string", "description": "Feature name"},
],
}
return {"type": "integer"}
Same for FeatureStateValueEdgeIdentityField which accepts int | str | bool.
But overall looks good. I was moving on comparing with prod schemas, I ran it against your branch, here is the output if it can be of any help:
| Metric | Production | This PR |
|--------------------|------------|---------|
| OpenAPI Version | 2.0 | 3.0.3 |
| Endpoints | 524 | 509 |
| Schemas | 257 | 373 |
| Schemas with anyOf | 0 | 8 |
Critical fields still missing anyOf:
| Field | Production | This PR | Expected |
|---------------------|------------------------|----------------|-------------------------|
| feature | type: "integer/string" | type: "string" | anyOf: [int, str] |
| feature_state_value | type: "string" | type: "string" | anyOf: [int, str, bool] |
The 8 schemas with anyOf are all Pydantic models (e.g., TraitModel). The DRF serializer fields (EdgeFeatureField, FeatureStateValueEdgeIdentityField) need explicit OpenApiSerializerFieldExtension classes.
Missing endpoints (16) - all expected:
- SAML (14) - enterprise SSO
- Licence (1) - enterprise
- Onboarding (1) - renamed
- Add missing developer documentation - Refresh CONTRIBUTING.md - Refresh the PR template - Import developer docs and CONTRIBUTING.md to docs.flagsmith.com
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Matthew Elwell <matthew.elwell@flagsmith.com>
- Added diff-cover as a dev dependency - Excluded app/settings/local.py from coverage - Added test for `resolve_pydantic_schema` - Added test for `get_edge_identity_pagination_parameters` - Added test for schema generation scenario (early return when no view context is provided)
7c01cc4 to
1e9902e
Compare
matthewelwell
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've given this a fairly surface review but LGTM.
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature!Changes
Contributes to #6183.
In this PR, we switch from drf-yasg to drf-spectacular for the OpenAPI/Swagger generation. This enables us to use
anyOf,oneOfandtype: str[]to express union type fields.Additionally, I've added an
sdktag to distinguish the SDK endpoints from the management ones.How did you test this code?
$.components.schemas.TraitModel.properties.trait_value.openapi-generator generate -i http://localhost:8000/api/v1/swagger.json\?format\=json -g typescript.