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

feat: post_create and post_generate hooks #667

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

iloveitaly
Copy link
Contributor

Description

Two additional hooks:

  • post_create. Helpful for writing custom logic to run after a object is fully generated. There's not a great place to put this logic right now.
  • post_generate. There are field-level post_generate methods, but nothing that can use the fully-generated model kwargs and run custom logic

I can write some additional tests for this, but wanted to get some initial feedback first.

@iloveitaly iloveitaly requested a review from guacs as a code owner March 20, 2025 21:34
@adhtruong
Copy link
Collaborator

Can the above functionality be achieved by overriding build and process_kwargs directly?

@iloveitaly
Copy link
Contributor Author

Can the above functionality be achieved by overriding build and process_kwargs directly?

It can, but:

  1. Overriding build requires you to call super() properly and type the params specific to the factory base class you are using
  2. Overriding either of these requires understanding the internals of polyfactory. Using either of the proposed hooks requires no knowledge of polyfactory or changes as polyfactory continues to change
  3. It's not self-documenting. Providing specific hooks makes it more clear exactly what you are trying to do

Copy link
Collaborator

@adhtruong adhtruong left a comment

Choose a reason for hiding this comment

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

The above feels like could be handled with docs if wanted to handle that case? Not opposed to adding though depending what other @litestar-org/maintainers think.

return cls.post_generate(result)

@classmethod
def post_create(cls, model: T) -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can this be called post_build? create is used to mean used with persistance handlers

:param model: The created model instance.
"""
pass
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can this return the instance to be consistent with post_generate?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Do these need to be called on coverage methods too?

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.

2 participants