factory_boy icon indicating copy to clipboard operation
factory_boy copied to clipboard

Published 20 hours ago • verified publisher icon FactoryBoy

Add Async Factory Support

Open nadege opened this issue 4 years ago • 29 comments

Hello,

I heard you may be interested in a PR to add support for async Factories, related issue : #679

We use the solution I documented in the issue in a project at work (aiohttp + aiopg) and it works well, but even if we have models with some complexity (lot of Subfactory themselves using SelfAttribute), I know we don't use a lot of features of Factory Boy...

For this PR I chose to add a new strategy for async creation, and added a new class AsyncFactory that default to this strategy. I also added a few methods to class Factory, and functions to the helpers module, to create, batch create, generate, batch generate... with this new strategy. There should be no issue with retro-compatibility, it only add new features.

I wrote some tests and documentation but they may be insufficient. Advice on what to test or document further is welcome of course.

Duplicating all tests to run them with an async factory seems a bit overkill to me, but maybe we should for some test cases ?

what do you think ?

nadege avatar Oct 30 '20 17:10 nadege

Thanks for the review! I'll fix the simpler things first (doc, tests..) and then try to add an implementation of _create_model_async for a library.

nadege avatar Nov 04 '20 15:11 nadege

I added an implementation for https://github.com/encode/orm ORM in the last commit. It's the best I found which support async and sqlite. I don't know how to simply add a postgresql database to the current CI, but when it'll be replaced by github actions it should be easier. I'll work on an implementation for a PG backed ORM then.

Last problem: unittest.IsolatedAsyncioTestCase is new in python3.8 I need to find a solution for previous python versions.

(I also have documentation spelling errors.)

nadege avatar Nov 27 '20 16:11 nadege

I resolved the compatibility issues with older python versions and spelling errors in the doc. Should be ready for a new review

nadege avatar Dec 04 '20 16:12 nadege

oh I still need to update /docs/reference.rst, the release note and CREDIT files.

nadege avatar Dec 05 '20 11:12 nadege

I'll wait for djangoproject.com to be fixed so I can run make doc before pushing my next commit. Also that's why CI failed on my last commit.

nadege avatar Dec 05 '20 13:12 nadege

Issues highlighted in previous review should be fixed. Added doc, credits and changelog. Ready for next review :)

nadege avatar Dec 11 '20 15:12 nadege

Hi, @nadege . Great work you've done. However, I would like to ask will you plan to add async support for post_generation methods ? I haven't found any code changes related to that feature.

TheLazzziest avatar Dec 26 '20 17:12 TheLazzziest

Hello ! Ok I found time to continue work on this. I'll look into the post generation feature. I think I never used that before.

nadege avatar Jan 15 '21 09:01 nadege

I handled the remarks you made in the last review. Now I'll look into post-generation

nadege avatar Jan 15 '21 16:01 nadege

Seems that LazyAttribute that depends on an async SubFactory receives a Task instead of an object.

mdczaplicki avatar Jan 18 '21 14:01 mdczaplicki

@mdczaplicki would you have a test that reproduce the issue ?

nadege avatar Jan 22 '21 10:01 nadege

Sure, I'll provide you with something next week. :)

On Jan 22 2021, at 11:06 am, Nadege Michel [email protected] wrote:

@mdczaplicki (https://github.com/mdczaplicki) would you have a test that reproduce the issue ? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub (https://github.com/FactoryBoy/factory_boy/pull/803#issuecomment-765293517), or unsubscribe (https://github.com/notifications/unsubscribe-auth/ACFPY2QYTE2IMOMIRBU7NE3S3FE2DANCNFSM4TFHREYQ).

mdczaplicki avatar Jan 22 '21 15:01 mdczaplicki

@nadege @mdczaplicki Any progress on this?

fadedDexofan avatar Feb 21 '21 12:02 fadedDexofan

No, I'm between jobs and my personal computer just broke, so I can't work on this at the moment.

nadege avatar Feb 24 '21 11:02 nadege

Hey @nadege the work here is awesome! 🥇 I am really interested on having this feature kicked out of the door, do you have any plans to finish it? If you don't mind I would be happy to jump in and help to get it done. Could you tell what is missing?

romulorosa avatar Jun 30 '21 11:06 romulorosa

IIRC, the main piece remaining is deciding how to handle the post-generation hooks.

francoisfreitag avatar Jun 30 '21 11:06 francoisfreitag

hi @romulorosa ! I don't have much time (or energy :x) to continue this short term. So I'd be happy to see you take it from here !

nadege avatar Jun 30 '21 13:06 nadege

any updates on this PR?

gegnew avatar Nov 01 '22 17:11 gegnew

IIRC, the main piece remaining is deciding how to handle the post-generation hooks.

@francoisfreitag Can this feature be released in parts? As in, first release the basic functionality and then add the post-generation-hooks in a future release.

ShipraShalini avatar Nov 04 '22 11:11 ShipraShalini

The post generation hooks need handling. The handling can probably be to raise NotImplemented when a post generation hook is declared on an AsyncFactory. Although clearly not ideal, that would at least improve the use case for factory boy in an async context.

francoisfreitag avatar Nov 04 '22 11:11 francoisfreitag

My use case doesn't require post-generation-hooks, so I can work on getting it out with raise NotImplemented.

ShipraShalini avatar Nov 04 '22 15:11 ShipraShalini

I'd like to see this PR get merged too. @ShipraShalini did you make some progress there on the raise NotImplemented ? Can I offer assistance?

coneybeare avatar Jan 05 '23 16:01 coneybeare

Hello there, I'm back 🙇

Changes on the PR since last time (3 years ago, time flies... 👵)

  • [x] 😅 Rebase
  • [x] 🔄 Replaced orm implementation by a SQLAlchemy one (with sqlite only)
  • [x] 🔥 Dropped python 3.6 support. Now using asyncio.run.
  • [x] 🛠️ Fix lint & other CI failures

TODO:

  • [x] 🧪 One test for Post Generation hook.
  • [x] ❓ Decide what to do about post generation. (NotImplemented exception, handle it...)
  • [ ] 🧪 More tests? (please advise!)

Dropped:

  • async implem in sqlalchemy app works with postgres. I tried to use asyncpg, the test were stuck in "teardown" of asyncs tests in test_alchemy.py.

nadege avatar Jul 22 '23 10:07 nadege

And, post generation is now handled too! 🎉

I played with the suggestion from @francoisfreitag, to use add_done_callback. Now the whole post generation step is a callback when dealing with a Task.

Code is in last commit, it's a bit quick & dirty. Not sure of the cleanest way to declare the _handle_post_generation function. Happy to make any change or add more tests. Please advise 🙇

nadege avatar Jul 23 '23 10:07 nadege

This is awesome @nadege. I hope it gets merged soon, it's been a few months already since your last commit.

matinone avatar Dec 05 '23 22:12 matinone

Great work @nadege , it was very helpful in fixing some of the issues with the solution in my team. I also hope this gets merged soon so we can migrate away from temporary workarounds :)

mlorenc-sonnen avatar Dec 06 '23 14:12 mlorenc-sonnen

Appreciate all the work on this @nadege. :-)

@francoisfreitag is there other work needed to get this merged? Thanks!

nextmat avatar Jan 04 '24 17:01 nextmat

I rebased & addressed the simpler comments, realised we were not committing/flushing so I fixed that.

TODO:

  • [ ] test that we actually save objects in slqalchemy implementation
  • [ ] use IsolatedAsyncioTestCase in tests
  • [ ] handle get_or_create in async implem for sqlalchemy?

I can find time for first 2 items.

Edit: Thank you and congrats for the baby! 😁

nadege avatar Jan 13 '24 10:01 nadege