search

OCA / storage

GNU Affero General Public License v3.0
70 stars 159 forks source link

Python 3.9 fixes, auth option for webdav, fixed stored filename #285

Closed wpichler closed 9 months ago

wpichler commented 1 year ago

Hi all,

three small fixes for some small problems...

[FIX] Small fix to reformat the auth option as tuple for protocol webdav. It is not possible to specify a tuple in json ! [FIX] Fixed the stored filename to include the path. Without this you are not able to store files in custom sub directories [FIX] Added future import annotations to be python 3.9 compatible

OCA-git-bot commented 1 year ago

Hi @lmignon, some modules you are maintaining are being modified, check this out!

lmignon commented 12 months ago

@wpichler To move forward with the 2 validated commits regarding the support for python3.9 and the fix for webdav, I cherry picked the two commits into #286, fixed pre-commit and added a release note. Your 2 fixes will be included into the next release.

lmignon commented 12 months ago

The part regarding the path preservation has to be refined. (unittests, ...)

len-foss commented 9 months ago

Hello Laurent!

First, this is a great implementation of the storage concept and I hope there will be little issue with porting modules.

It's been working like a charm, except for the paths issue, which breaks everything.

I started working on an implementation of your proposed solution, putting that code snippet in a function. In our test case we had a rooted directory part before the filesystem path - basically for a thumbnail the rooted path is "odoo/attachments" and the added path will be something like "53/2b". But, it turns out that the 'rooted_path' was never a problem in our tests. In other words, the diff you suggest, although more complicated, does exactly the same thing.

Are you fine with the simple diff if there's a test case checking for the rooted path issue?

len-foss commented 9 months ago

By the way @wpichler, actually commit ef68e0df9145c3f5e89445e8b803e5ac64edc014 wasn't necessary, you could simply state: {"base_url": "https://username:password@host.tld/remote.php/webdav"} As you can see in the documentation: https://www.python-httpx.org/advanced/#authentication

Although it's not very intuitive so it might be good to have some doc in the future.

lmignon commented 9 months ago

Are you fine with the simple diff if there's a test case checking for the rooted path issue?

No sure what you suggest. Nevertheless a test case with the fix has more chance to be merged than a fix without tests :smirk: