[17.0][IMP] fs_attachment_s3: Optimize clean up storages using mass deletion#599
Conversation
|
Hi @lmignon, |
a596bc5 to
b858c7d
Compare
| - [APSL-Nagarro](https://apsl.tech): | ||
| - Antoni Marroig \<<amarroig@apsl.net>\> |
There was a problem hiding this comment.
I would be nice to follow the same pattern as for others contributors ;-)
b858c7d to
964ac2f
Compare
|
This PR has the |
|
Seems it's ready for merge @lmignon ? |
| "GC: Sending bulk delete request to S3 (%s files)", | ||
| len(objects_to_delete), | ||
| ) | ||
| s3_client.delete_objects( |
There was a problem hiding this comment.
Here you call an async IO method outside a coroutine. That's mean that the method will never be awaited and then execute....
fsspec.asyn.sync(
fsspec.asyn.get_loop(),
s3_client.delete_objects,
Bucket=bucket_name,
Delete={"Objects": objects_to_delete},
)There was a problem hiding this comment.
Done, could you review again please?
964ac2f to
51cfad6
Compare
|
@peluko00 Why do you copy paste my code in your changes? A cherry -pick would have preserved the authorship as I did it on the 16.0 branch with your commit. It will also makes conflicts on back/fw port from/to 16.0 |
s3fs uses aiobotocore as s3 client, which is asynchronous, but Odoo does not support async calls, therefore the call to delete objects from S3 was not working. The fsspec library provides a way to run async calls in a synchronous way. The fsspec.asyn.sync function is now used to run the async call to delete objects from S3, in a synchronous way, in order to make it work in Odoo.
cd1a31e to
0ccc238
Compare
Sorry about that, now cherry-pick is done! |
|
Could you merge please @lmignon ? |
|
/ocabot merge minor |
|
What a great day to merge this nice PR. Let's do it! |
|
Congratulations, your PR was merged at d559e1b. Thanks a lot for contributing to OCA. ❤️ |
4 participants
Optimize clean up storages using mass deletion with S3
cc https://github.com/APSL 38655
@mpascuall @BernatObrador @ppyczko @shirashi3771 @palomagrc93 @miquelalzanillas @LlucSG please review