Skip to content
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

load_default reusing property instance between loaded instances #2156

Open
sebarys opened this issue Jul 27, 2023 · 3 comments
Open

load_default reusing property instance between loaded instances #2156

sebarys opened this issue Jul 27, 2023 · 3 comments
Labels

Comments

@sebarys
Copy link

sebarys commented Jul 27, 2023

Hi

I've defined following schema for my HTTP request (use flask as http server)

class Nested:
    def __init__(self, msg: str, variables: dict[str, str]):
        self.msg = msg
        self.variables = variables


class NestedSchema(BaseSchema):
    msg = fields.String(required=True, validate=[validate.Length(min=1)])
    variables = \
        fields.Dict(keys=fields.Str(), values=fields.Str(), allow_none=True, required=False, load_default={})

    @post_load
    def make(self, data, **kwargs) -> Nested:
        return Nested(**data)

class RequestPayload:
    def __init__(self, nested: Nested, score: Optional[float]):
        self.nested = nested
        self.score = score


class RequestPayloadSchema(BaseSchema):
    nested = fields.Nested(NestedSchema, required=True, allow_none=False)
    score = fields.Float(allow_none=True,
                               validate=[validate.Range(min=0.0, min_inclusive=True, max=1.0, max_inclusive=True)],
                               required=False,
                               load_default=None
                               )

    @post_load
    def make(self, data, **kwargs) -> RequestPayload:
        return RequestPayload(**data)

When testing application locally I've noticed super strange behaviour: for subsequent HTTP requests when variables is not provided in request payload I sometimes see that it is not empty dict.

It looks that load_default re-use instance so when during internal processing/some request provide value for it, requests without this value won't have empty dict but contain value from different request.

Is it a way to make sure that default values will always be as defined (not share mutable instance)?

@lafrech
Copy link
Member

lafrech commented Jul 27, 2023

This is the dreaded Python "mutable default argument" pitfall. Search here or on the Internet.

@sebarys
Copy link
Author

sebarys commented Jul 27, 2023

@lafrech thanks for quick answer!

I know that Python have such odd behaviour, but for serialisation use case can't library under the hood prepare deepcopy for each time load_default is used?

I don't see at first glance any use cases when it is desired so maybe it would be good to have reasonable default behaviour?

@lafrech
Copy link
Member

lafrech commented Jul 27, 2023

Up till now our stance has been "pass a callable". Most of the time, it's gonna be list or dict and lambda functions make for more complete cases. Apart from users being caught by it, it has never been such a blocker we felt the need to protect the user (at the cost of a performance impact, be it minimal).

This can always be reconsidered, though. But is it worth it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants