feat: Capability for Pydantic-based OpenAPI response schemas #3795

khvn26 · 2024-04-18T08:12:48Z

Thanks for submitting a PR! Please check the boxes below:

I have run pre-commit to check linting
I have added information to docs/ if required so people know about the feature!
I have filled in the "Changes" section below?
I have filled in the "How did you test this code" section below?
I have used a Conventional Commit title for this Pull Request

Changes

This PR solves incorrect rendering of Optional fields of the environment document Swagger spec, and adds a generalised way to use Pydantic models to describe API responses with swagger_auto_schema.

Add exclude_model_fields utility function
Add PydanticResponseCapableSwaggerAutoSchema drf-yasg view inspector
Fix invalid nullable field schema generation

How did you test this code?

Observed correct Swagger schema locally.
Added a unit test.

vercel · 2024-04-18T08:12:55Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Apr 25, 2024 8:15pm
flagsmith-frontend-preview	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Apr 25, 2024 8:15pm
flagsmith-frontend-staging	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Apr 25, 2024 8:15pm

github-actions · 2024-04-18T08:54:26Z

Uffizzi Ephemeral Environment Deploying

☁️ https://app.uffizzi.com/github.com/Flagsmith/flagsmith/pull/3795

⚙️ Updating now by workflow run 8817120325.

What is Uffizzi? Learn more!

matthewelwell

Added some general comments and also tested the swagger auto generation by running the code locally and viewing the generated docs.

api/util/pydantic.py

api/api/openapi.py

codecov-commenter · 2024-04-22T11:42:23Z

Codecov Report

Attention: Patch coverage is 98.07692% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 95.92%. Comparing base (507080e) to head (ec2eed3).
Report is 6 commits behind head on main.

❗ Current head ec2eed3 differs from pull request most recent head 11c2d21. Consider uploading reports for the commit 11c2d21 to get more accurate results

Files	Patch %	Lines
api/api/openapi.py	96.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3795      +/-   ##
==========================================
- Coverage   95.96%   95.92%   -0.05%     
==========================================
  Files        1126     1125       -1     
  Lines       35786    35530     -256     
==========================================
- Hits        34342    34081     -261     
- Misses       1444     1449       +5

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

matthewelwell · 2024-04-22T13:13:41Z

api/api/openapi.py

+        return elem
+
+
+class PydanticResponseCapableSwaggerAutoSchema(SwaggerAutoSchema):


Since this is the entry point to this logic for anyone coming across the class reference in the settings file, I think it would be good to add a docstring to this class with a short summary explaining why we're overriding the default behaviour.

zachaysan

The rest of it looks 👌🏼 but I'm concerned about type not being set as intentioned. Assuming that is fixed feel free to merge this PR.

zachaysan · 2024-04-25T18:30:27Z

api/api/openapi.py

+        definitions = self.components.with_scope(SCHEMA_DEFINITIONS)
+
+        for status_code in list(response_serializers):
+            if isinstance(response_serializers[status_code], type) and issubclass(


I believe this if check will always fail since type is being passed in as a built-in function.

type is, in fact, a class:

>>> type <class 'type'> >>> type.__mro__ (<class 'type'>, <class 'object'>)

So the instance check works as intended:

>>> obj = object() >>> isinstance(obj, type) False >>> class Class: pass ... >>> isinstance(Class, type) True

Additionally, you can see the coverage report to make sure the code path is being run.

Well, it's good to see that type is set as intentioned, but it's a surprising check that at least warrants a comment so that confused developers are set right.

Changed to inspect.isclass for more clarity.

- Add `exclude_model_fields` utility function - Add `PydanticResponseCapableSwaggerAutoSchema` drf-yasg view inspector - Fix invalid nullable field schema generation

khvn26 requested a review from matthewelwell April 18, 2024 08:12

github-actions bot added the api Issue related to the REST API label Apr 18, 2024

vercel bot deployed to Preview – flagsmith-frontend-preview April 18, 2024 08:25 View deployment

vercel bot deployed to Preview – docs April 18, 2024 08:25 View deployment

vercel bot deployed to Preview – flagsmith-frontend-staging April 18, 2024 08:27 View deployment

khvn26 force-pushed the feat/pydantic-response-schemas branch from 8df4d0a to 8024333 Compare April 18, 2024 08:46

vercel bot deployed to Preview – flagsmith-frontend-staging April 18, 2024 08:49 View deployment

vercel bot deployed to Preview – docs April 18, 2024 08:49 View deployment

vercel bot deployed to Preview – flagsmith-frontend-preview April 18, 2024 08:51 View deployment

khvn26 force-pushed the feat/pydantic-response-schemas branch from 8024333 to 63b244f Compare April 18, 2024 08:51

vercel bot deployed to Preview – flagsmith-frontend-preview April 18, 2024 08:53 View deployment

vercel bot deployed to Preview – flagsmith-frontend-staging April 18, 2024 08:55 View deployment

vercel bot deployed to Preview – docs April 18, 2024 08:56 View deployment

matthewelwell reviewed Apr 22, 2024

View reviewed changes

api/util/pydantic.py Show resolved Hide resolved

api/api/openapi.py Show resolved Hide resolved

api/api/openapi.py Show resolved Hide resolved

api/api/openapi.py Outdated Show resolved Hide resolved

api/api/openapi.py Show resolved Hide resolved

khvn26 force-pushed the feat/pydantic-response-schemas branch from 1c06394 to f3b132f Compare April 22, 2024 11:44

khvn26 requested a review from matthewelwell April 22, 2024 11:45

vercel bot deployed to Preview – flagsmith-frontend-staging April 22, 2024 11:47 View deployment

vercel bot deployed to Preview – flagsmith-frontend-preview April 22, 2024 11:49 View deployment

vercel bot deployed to Preview – docs April 22, 2024 11:50 View deployment

matthewelwell reviewed Apr 22, 2024

View reviewed changes

khvn26 force-pushed the feat/pydantic-response-schemas branch from f3b132f to 415c925 Compare April 24, 2024 13:03

khvn26 requested a review from matthewelwell April 24, 2024 13:03

khvn26 force-pushed the feat/pydantic-response-schemas branch from 415c925 to ec2eed3 Compare April 24, 2024 13:08

vercel bot deployed to Preview – flagsmith-frontend-preview April 24, 2024 13:25 View deployment

vercel bot deployed to Preview – docs April 24, 2024 13:26 View deployment

vercel bot deployed to Preview – flagsmith-frontend-staging April 24, 2024 13:28 View deployment

zachaysan approved these changes Apr 25, 2024

View reviewed changes

khvn26 added 6 commits April 25, 2024 21:08

feat: Capability for Pydantic-based OpenAPI response schemas

59ecd28

- Add `exclude_model_fields` utility function - Add `PydanticResponseCapableSwaggerAutoSchema` drf-yasg view inspector - Fix invalid nullable field schema generation

Remove jsonref

3162f3e

Add test, fix iteration error

62833b8

address comments

6ddcf36

add docs

b23cab4

address comments

11c2d21

khvn26 force-pushed the feat/pydantic-response-schemas branch from 3fdf3a8 to 11c2d21 Compare April 25, 2024 20:08

khvn26 added this pull request to the merge queue Apr 25, 2024

Merged via the queue into main with commit 609deaa Apr 25, 2024
18 of 21 checks passed

khvn26 deleted the feat/pydantic-response-schemas branch April 25, 2024 20:08

dabeeeenster mentioned this pull request Apr 25, 2024

chore(main): release 2.111.0 #3849

Merged

vercel bot deployed to Preview – flagsmith-frontend-staging April 25, 2024 20:12 View deployment

vercel bot deployed to Preview – docs April 25, 2024 20:12 View deployment

vercel bot deployed to Preview – flagsmith-frontend-preview April 25, 2024 20:15 View deployment

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Capability for Pydantic-based OpenAPI response schemas #3795

feat: Capability for Pydantic-based OpenAPI response schemas #3795

khvn26 commented Apr 18, 2024

vercel bot commented Apr 18, 2024 •

edited

Loading

github-actions bot commented Apr 18, 2024 •

edited

Loading

matthewelwell left a comment

codecov-commenter commented Apr 22, 2024 •

edited

Loading

matthewelwell Apr 22, 2024

khvn26 Apr 24, 2024

zachaysan left a comment

zachaysan Apr 25, 2024

khvn26 Apr 25, 2024

zachaysan Apr 25, 2024

khvn26 Apr 25, 2024

		return elem


		class PydanticResponseCapableSwaggerAutoSchema(SwaggerAutoSchema):

feat: Capability for Pydantic-based OpenAPI response schemas #3795

feat: Capability for Pydantic-based OpenAPI response schemas #3795

Conversation

khvn26 commented Apr 18, 2024

Changes

How did you test this code?

vercel bot commented Apr 18, 2024 • edited Loading

github-actions bot commented Apr 18, 2024 • edited Loading

Uffizzi Ephemeral Environment Deploying

matthewelwell left a comment

Choose a reason for hiding this comment

codecov-commenter commented Apr 22, 2024 • edited Loading

Codecov Report

matthewelwell Apr 22, 2024

Choose a reason for hiding this comment

khvn26 Apr 24, 2024

Choose a reason for hiding this comment

zachaysan left a comment

Choose a reason for hiding this comment

zachaysan Apr 25, 2024

Choose a reason for hiding this comment

khvn26 Apr 25, 2024

Choose a reason for hiding this comment

zachaysan Apr 25, 2024

Choose a reason for hiding this comment

khvn26 Apr 25, 2024

Choose a reason for hiding this comment

vercel bot commented Apr 18, 2024 •

edited

Loading

github-actions bot commented Apr 18, 2024 •

edited

Loading

codecov-commenter commented Apr 22, 2024 •

edited

Loading