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

Fix nested schema context thread-safety #1833

Draft
wants to merge 1 commit into
base: dev
Choose a base branch
from
Draft

Conversation

lafrech
Copy link
Member

@lafrech lafrech commented Jun 22, 2021

Fixes #1617.
Closes #1791.

This fixes the thread-safety issue but might be a breaking change for people relying on the update: parent schema updates nested schema context. See #1791.

@lafrech
Copy link
Member Author

lafrech commented Jun 22, 2021

The alternative, to avoid breaking the update case, would be to document it as not thread-safe and invite the users to use a class rather than an instance, as this is thread-safe (but does not allow context update from parent, only replacement, and doesn't allow to use modifiers in the nested schema init).

Then in 4.0, drop the update case if this is acceptable. And perhaps implement #1826.

The update case allows users to specify a context at both parent and nested level. In practice, I'm not sure this is worth supporting but I could be proven wrong. Besides, the design smells a bit, as the update flattens parent and nested contexts without namespacing nested context. I hope I'm making sense.

@lafrech
Copy link
Member Author

lafrech commented Jul 21, 2021

See discussion in #1791.

It might be better not to merge this (breaking change) and instead perhaps raise a warning about the thread-safety issue.

@lafrech lafrech marked this pull request as draft November 8, 2021 23:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Context and instantiated Netsted schemas. Context is not thread-safe (at least) for nested scheme
1 participant