rest-framework
rest-framework copied to clipboard
OCA
[16.0][FIX] fastapi: Transactions handling refactoring
This change is a complete rewrite of the way the transactions are managed in the when integrating a fastapi application into Odoo.
In the previous implementation, specifics error handlers were put in place to catch exception occurring in the handling of requests made to a fastapi application and to rollback the transaction in case of error. This was done by registering specifics error handlers methods to the fastapi application using the 'add_exception_handler' method of the fastapi application. In this implementation, the transaction was rolled back in the error handler method.
This approach was not working as expected for several reasons:
- The handling of the error at the fastapi level prevented the retry mechanism to be triggered in case of a DB concurrency error. This is because the error was catch at the fastapi level and never bubbled up to the early stage of the processing of the request where the retry mechanism is implemented.
- The cleanup of the environment and the registry was not properly done in case of error. In the 'odoo.service.model.retrying' method, you can see that the cleanup process is different in case of error raised by the database and in case of error raised by the application.
This change fix these issues by ensuring that errors are no more catch at the fastapi level and bubble up the fastapi processing stack through the event loop required to transform WSGI to ASGI. As result the transactional nature of the requests to the fastapi applications is now properly managed by the Odoo framework.
@sbidoul @simahawk @qgroulard @AnizR @sebastienbeau This PR is a fundamental change in the way exceptions are handled in Fastapi's Odoo integration. It should be transparent but ensure a proper management of the transactions, environments and registry in case of exception. Before this change, for example, in some cases, some exceptions did not appear in the log files. This should no longer be the case, and the stack trace should be more relevant. The retry mechanism did not work as expected either.
Agree with the concept. I suppose you'll want to battle test it before declaring it ready to merge?
Agree with the concept. I suppose you'll want to battle test it before declaring it ready to merge?
The testsuite now ensures that the commit method is not called in the case of an exception, unlike normal calls. I'm going to do some additional tests on my project but by running all the tests in the debugger I've been able to check that whatever exception is thrown, it goes back into Odoo's retrying method which manages the transactional aspect of the calls. The dispatcher also handles the formatting of the response according to the type of error.
~~Found issue.... will fix~~
Issue no in the base lib but in my endpoint implementation.... a 'return HttpException in place of araise HttpException :face_with_spiral_eyes:
On my way to merge this fine PR! Prepared branch 16.0-ocabot-merge-pr-422-by-lmignon-bump-minor, awaiting test results.
Congratulations, your PR was merged at a64cfa72b3f80edafec757c9f4b4de330c8a8c70. Thanks a lot for contributing to OCA. ❤️