RFC storage_backend: keep files unavailable while uploading (in ftp / sftp)

bealdav commented 4 years ago

Case

Customer cases need to ensure that an uploading file in at least sftp/ftp (probably other backends) can't be consumed before to be fully uploaded.

Do you think this feature is generic ?

Should it be implemented in this repo ?

I think yes

Possible way to do it

It seems that the easier way to do it is to upload the file with a temporary name and when the file is completed, to rename it. Consumers of these files should have filters to exclude files like pattern argument in https://github.com/OCA/storage/blob/12.0/storage_backend/models/storage_backend.py#L46. It's the only inconvenient, but I think it's acceptable

More sophisticated and flexible manners could be implemented if resource are available for it in a second time.

How to plug it in existing code

I don't have knowledge enough in this repo to find the better place

I wait for expert advices

What pattern to use to make files unavailable

use a dot prefix .myfile.xml
a suffix 'myfile.xml.tmp'
any other

Thanks to share your comments

@sebastienbeau @florian-dacosta @simahawk @lmignon @GSLabIt @Cedric-Pigeon

florian-dacosta commented 4 years ago

I think it should go in storage_backend. I like the ".tmp" suffix idea, it could be an option.

Later it could be done with a tmp directory, creating the file in the tmp dir and then move it once it is done.

simahawk commented 4 years ago

Sounds like a nice idea. I would add a new field on the backend ignore_file_pattern and use it where needed. Question: what happens w/ "get"? If we don't filter it the user will be able to see the file anyway and - considering the scope of shopinvader - the search engine will index its path and the website will load it. At the same time, if you filter "get" you'll prevent users to see the file in odoo backend. Maybe we need something at storage file level (a state or a flag) to state when the file is ready or simply filter out files w/ the same pattern on the search engine.

sebastienbeau commented 4 years ago

Hi I think also that we should put this here.

What we can do is

implementing a rename method on the storage (we can take this opportunity to implement the move method too)
add a params on the _add_b64_data and _add_bin_data "use_temporary_file=True/False" (False by default )
then when we add a file and if this option is pass we add the file with a temporary name and do a rename when the export is finish.

Note : not all implementation backend can support this, for example it's not possible with S3 to do this (renaming is not supported by the API), so in this case if the option is pass we should raise an error.

Note 2: this do not have any impact in the context of shopinvader as we have 2 step

storage_image module just add image and then is done it commit the data in the database
the url are pushed into the search engine (the image are already sync)

We only need this feature when we have a process of import/export file with an external system that can read the file before it's ready

github-actions[bot] commented 2 years ago

There hasn't been any activity on this issue in the past 6 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. If you want this issue to never become stale, please ask a PSC member to apply the "no stale" label.

OCA / storage