Skip to content

feat: post_create and post_generate hooks #667

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

Open
wants to merge 3 commits 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.

: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
Contributor Author

Choose a reason for hiding this comment

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

Yes! I can make that change.

I'm personally not a fan of returning when we don't need to as it hints to the developer that for some reason we need to return and therefore the value is used for something

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, good point. Will add.

Copy link
Collaborator

@adhtruong adhtruong Apr 14, 2025

Choose a reason for hiding this comment

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

Based on this repetition it makes sense to add as need to repeat logic otherwise. Let me know if need anything clarity on other comments!

Copy link

Documentation preview will be available shortly at https://litestar-org.github.io/polyfactory-docs-preview/667

@iloveitaly
Copy link
Contributor Author

@adhtruong addressed PR feedback!

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