Skip to content

Enhancement: Adds SecretStr, Path, and Enum to built in serializer #472

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

Merged
merged 4 commits into from
Sep 10, 2022
Merged

Enhancement: Adds SecretStr, Path, and Enum to built in serializer #472

merged 4 commits into from
Sep 10, 2022

Conversation

cofin
Copy link
Member

@cofin cofin commented Sep 10, 2022

In the interest of not leaving this open too long, I'm submitting with the initial ones and a framework that we can add additional ones to relatively quickly. Let me know if this look ok

Resolves #458

cofin and others added 2 commits September 10, 2022 12:33

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@cofin cofin added the Enhancement This is a new feature or request label Sep 10, 2022
@lgtm-com
Copy link

lgtm-com bot commented Sep 10, 2022

This pull request introduces 1 alert when merging 6ceabd3 into 9452390 - view on LGTM.com

new alerts:

  • 1 for Unused import

@Goldziher Goldziher merged commit f767181 into litestar-org:main Sep 10, 2022
@cofin cofin deleted the updated-default-response branch September 10, 2022 19:30
@@ -74,6 +75,10 @@ def serializer(value: Any) -> Dict[str, Any]:
"""
if isinstance(value, BaseModel):
return value.dict()
if isinstance(value, SecretStr):
Copy link

Choose a reason for hiding this comment

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

@cofin is this ordering correct?
wouldn't you want the SecretStr to take priority over all other types?

Copy link
Contributor

Choose a reason for hiding this comment

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

why?

@dannya
Copy link

dannya commented Sep 12, 2022 via email

@Goldziher
Copy link
Contributor

Is it possible that someone passes in an object that extends both BaseModel and SecretStr?

On Mon, 12 Sep 2022, at 7:15 PM, Goldziher wrote:

@.**** commented on this pull request.

In starlite/response.py #472 (comment):

@@ -74,6 +75,10 @@ def serializer(value: Any) -> Dict[str, Any]:
"""
if isinstance(value, BaseModel):
return value.dict()

  •    if isinstance(value, SecretStr):
    

why?


Reply to this email directly, view it on GitHub #472 (comment), or unsubscribe https://github.com/notifications/unsubscribe-auth/AADALP6HZC3MW7O3VUORW5LV55XL5ANCNFSM6AAAAAAQJNLS2U.
You are receiving this because you commented.Message ID: @.***>

I can't see how it is tbh.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement This is a new feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enhancement: Add more types to the default Response serializer.
3 participants