[16.0][MIG] storage_backend_s3: Migration to 16.0

[xavierpiernas]

No description provided.

[xavierpiernas]

Hello maintainers and community,

We've worked on migrating the storage_backend_s3 addon to Odoo 16.0.

Changes made:

1. Tests have been adapted to use setUpClass.
2. vcrpy-unittest has been added as a dependency and requirement.

I kindly ask for a review of these changes and any feedback is greatly appreciated.

Thank you for your time and effort.

[OriolMForgeFlow]

Code review: LGTM!

[lmignon]

@xavierpiernas Why do you need storage_backend_s3 ? The same functionalities and even more are available within the new sf_storage addon... #252 This PR is ready for review. We use it to store our odoo attachments in S3 for our odoo 16 project via #260 and the rewrite of the storage_file addons is started and based on this new approach.

[xavierpiernas]

@xavierpiernas Why do you need storage_backend_s3 ? The same functionalities and even more are available within the new sf_storage addon... #252 This PR is ready for review. We use it to store our odoo attachments in S3 for our odoo 16 project via #260 and the rewrite of the storage_file addons is started and based on this new approach.

We have tested the suggested app and we have noticed that images uploaded to website are also stored in S3 but the preview is not working. Is this a normal behaviour? @lmignon

[lmignon]

We have tested the suggested app and we have noticed that images uploaded to website are also stored in S3 but the preview is not working. Is this a normal behaviour? @lmignon

@xavierpiernas Yes it's the normal behaviour. I'm developping a new addon dedicated to the store of images into a filesystem storage (#274) It defines a new kind of field sf_image working as the Image field into odoo but with the goodies of the fs_file field. The widget should be added in the coming days and also a mixin as the one defined into the storage_thumbnail. With this new approach, the definition of the mixin will be as easy as writing:

from odoo import models
from odoo.addons.fs_image.fields import FsImage


class FsImageMixin(models.AbstractModel):
    _name = 'fs.image.mixin'
    _description = "FS Image Mixin"

    image_1920 = FsImage("Image", max_width=1920, max_height=1920)

    # resized fields stored (as attachment) for performance
    image_1024 = FsImage("Image 1024", related="image_1920", max_width=1024, max_height=1024, store=True)
    image_128 = FsImage("Image 128", related="image_1920", max_width=128, max_height=128, store=True)

This new field also allows you to set and get the alt_text (which replace the alt_name field in the storage_image addon) on the field itself. record.fs_image.alt_text = "my alt text" print(record.fs_image.alt_text

The main question here is to know how many resolutions we want to store by default.

• In odoo the main image size is limited to 1920 and copies are created for 1024, 512, 256, and 128
• In storage_thumbnail, there is no limitation in size for the main image but we've copies for 128 et 64.

In odoo 16 only the sizes 1920 and 1024 and 128 are used into the web and if even if you declare the an Image wihtout max_width and max_height, it will be limited by the system parameter base.image_autoresize_max_px to 1920x1920
The sizes 512 and 256 seems to be mainly for the website.

[prgueza]

@lmignon

Would it not be possible to allow Odoo to resize these images on the fly whenever the size parameters are specified in the request?

Currently with the version we have installed, requests such as /web/image/<string:xmlid>/<int:width>x<int:height> throw the following error:

2023-08-29 14:18:53,119 116 ERROR odoo odoo.http: Exception during request handling.
Traceback (most recent call last):
  File "/srv/odoo/odoo/parts/odoo/odoo/http.py", line 1998, in __call__
    response = request._serve_db()
  File "/srv/odoo/odoo/parts/odoo/odoo/http.py", line 1584, in _serve_db
    return service_model.retrying(self._serve_ir_http, self.env)
  File "/srv/odoo/odoo/parts/odoo/odoo/service/model.py", line 133, in retrying
    result = func()
  File "/srv/odoo/odoo/parts/odoo/odoo/http.py", line 1611, in _serve_ir_http
    response = self.dispatcher.dispatch(rule.endpoint, args)
  File "/srv/odoo/odoo/parts/odoo/odoo/http.py", line 1725, in dispatch
    return self.request.registry['ir.http']._dispatch(endpoint)
  File "/tmp/addons/website/models/ir_http.py", line 235, in _dispatch
    response = super()._dispatch(endpoint)
  File "/srv/odoo/odoo/parts/odoo/odoo/addons/base/models/ir_http.py", line 154, in _dispatch
    result = endpoint(**request.params)
  File "/srv/odoo/odoo/parts/odoo/odoo/http.py", line 697, in route_wrapper
    result = endpoint(self, *args, **params_ok)
  File "/tmp/addons/web/controllers/binary.py", line 144, in content_image
    stream = request.env['ir.binary']._get_image_stream_from(
  File "/srv/odoo/odoo/parts/odoo/odoo/addons/base/models/ir_binary.py", line 245, in _get_image_stream_from
    stream.size = len(stream.data)
TypeError: object of type 'NoneType' has no len()

I've tracked down the issue up to this point and I assume it is because the Stream object that we are creating does not have the data property set to a value that can be processed by the image_process function.

This Stream object is created via the _get_image_stream_from (ir_binary) -> _get_stream_from (ir_binary) -> _record_to_stream (fs_attachment.ir_binary) -> FsStream.from_fs_attachment (fs_attachment.ir_binary). The problem here is that I can't figure out how to include the data into the Stream at this point, when generating the stream. I don't even know if this is the best idea, but having the process_image function to process and return the stream data seems like the appropriate solution.

Thank you for your time!

[lmignon]

Would it not be possible to allow Odoo to resize these images on the fly whenever the size parameters are specified in the request?

@prgueza Is see how and where I could solve the problem you report. I'll keep you informed once it's done. Nevertheless you've to keep in mind that when you ask odoo to resize images on the fly, you consume cpu time and memory. The streaming benefits are also lost since the current implementation of the resize process need to get the image in memory and return the result as in memory bytes....

[prgueza]

I'll keep you informed once it's done

@lmignon Thank you very much, you are a lifesaver!

Nevertheless you've to keep in mind that when you ask odoo to resize images on the fly, you consume cpu time and memory. The streaming benefits are also lost since the current implementation of the resize process need to get the image in memory and return the result as in memory bytes....

I completely agree! This is not ideal for us and we are also looking for better ways to do this, the solution you proposed here sounds perfect for us! However, I think that crashing like it does now is not ideal either, I'd much rather have it working while we look for a better alternative than seeing errors like the one I commented about.

Something we are also looking into is being able to use the x-sendfile directive configuration for files stored in our AWS S3 bucket. We haven't managed to get it working. We are currently facing two problems, perhaps you could shed some light.

1. If fs is not configured for x_sendfile the placeholder image is served instead

When creating a Stream from the attachment, if x_sendfile is not configured for the fs or the attachment does not have a fs_url the from_fs_attachment method does not compute its size, which causes the _get_image_stream_from method from the IrBinary class to return the placeholder (here).

size = 0
if cls._check_use_x_sendfile(attachment):
    fs, _storage, fname = attachment._get_fs_parts()
    fs_info = fs.info(fname)
    size = fs_info["size"]
return cls(
    size=size <---- This is 0 when `cls._check_use_x_sendfile(attachment)` is False

And causes this to return the placeholder:

if not stream or stream.size == 0: <---- This is True
    if not placeholder:
        placeholder = record._get_placeholder_filename(field_name)
    stream = self._get_placeholder_stream(placeholder) <--- So Stream gets replaced
    if (width, height) == (0, 0):
        width, height = image_guess_size_from_field_name(field_name)

2. We are unsure about how to configure AWS S3 together with x-sendfile

We are unsure about which fs_url to use when working with a AWS S3 Bucket which is protected. In my head contents from this bucket are only accessible through the Odoo backend, which renders useless the x-sendfile purpose anyway. We have tried making the AWS S3 Bucket public via url, but setting fs_url to the base url of the bucket does not seem to work either. Do you know what we might be doing wrong here?

Thanks again for all your help! 😊

[lmignon]

@prgueza Thank you for all your feedback.
I've to admit that I never tested in real the x-sendfile at the moment since it's still not configured on our server (It's a pending task ...). Nevertheless, the first part of your comment is a bug also related to the missing part of the implementation of the image's streaming in fs_attachment.

Regarding the use and the configuration of x-sendfile. Do you've read the documentation here?
In short:

• The url to use must the url provided by internal_url field on the attachement.
• In your nginx configuration, you must configure a poxy location to serve all the files with an url prefixed with the code of your fs.storage instance

location /s3_prod/ {
 internal;
        internal;
        proxy_pass http://your_backend_server;
        proxy_set_header Authorization "Bearer YOUR_ACCESS_TOKEN"; # optional....
       ...
}

• internal: This directive ensures that the location block can only be accessed internally by Nginx.
• proxy_pass: Replace your_backend_server with the appropriate URL where your s3 server is listening.
• Authorization: Set this header with the appropriate credentials to access your s3 server if it's not public. It could be an access token or another form of authentication.

With x-senfile enable the following flow is generated.

When you ask for a file to odoo at /web/content/{rec.id}/{filename}{extension}
Odoo responds with dedicated headers 'X-Accel-Redirect' for nginx and 'X-Sendfile' for apache where the value is set to a path: /{fs_storage_code}{path_to_the_file_in_you_fs_storage}
The proxy detect this header and uses your configured location to get the file from your filesystem storage and serve it without redirecting the client to this server.

All this is the theory and you could maybe encounter a bug also into this part. As soon as I've time, I'll set up our server to use x-sendfile and will come back if I've some trouble.

Regards.

[prgueza]

Hi @lmignon!

Thanks for your response!

• Authorization: Set this header with the appropriate credentials to access your s3 server if it's not public. It could be an access token or another form of authentication.

I had read the documentation, but I hadn't though about the proxy_set_header option. It sounds like the way to go! We will give it a try and share with you the results.

We will be keeping an eye on your progress with the bug fixes and stability features for this module, thanks for your contributions! As of today we don't have the time nor the knowledge to contribute more than with user testing and bug reporting.

[lmignon]

Hi @prgueza

We will be keeping an eye on your progress with the bug fixes and stability features for this module, thanks for your contributions!

The bugs are now fixed in #274. The commit history has to be cleaned up and some specific unittests added.

As of today we don't have the time nor the knowledge to contribute more than with user testing and bug reporting.

Functional tests and good bug reports are valuable contributions.

Regards

[xavierpiernas]

@lmignon

Thank you for your hard work and dedication. We're currently in the process of testing the new fs_image module, and it's performing well in displaying image previews. However, we've encountered difficulties when attempting to migrate our existing attachments from the filestore.

We've implemented a scheduled action that executes the "force_storage" function, which appears to be the main one. Unfortunately, every time we initiate this process, our environment loses connection. It's possible that this issue is related to performance limitations in our development environment. We're planning to address this by allocating additional resources.

Additionally, we've experimented with a server action that executes the "_move_attachment_to_store" function for individual attachments and small sets. While it has proven effective for some, it does not work consistently for others.

We would greatly appreciate any insights or guidance you could provide to help us navigate this migration process and ensure a successful outcome.

[lmignon]

@xavierpiernas Do you've solved your problem? What's the status of this PR? If you want to have it merged, can you fix the conflicts?

Regards

[xavierpiernas]

Greetings @lmignon,

Good news! We've successfully migrated the attachments using the "fs_" apps. This means we no longer need the "storage_backend_s3". I think sticking with the "fs_" apps, especially with the new "fs_image," is a better option. So, I think we can close this PR.

Thanks a lot for your help.

Cheers.

[lmignon]

Thank you for your feedback @xavierpiernas .