TenantFileSystemStorage doesn't work anymore #249
Comments
|
@tomturner can you check that ? |
|
In my testing, starting with clean MEDIA_ROOT directory whichever tenant receives the first file upload ends up determining the directory path structure. So if I first upload to the public schema I get If I first upload to a schema called |
|
@atodorov i'm not behind the code but if i remember they removed path and url methods overides of FileSystemStorage in TenantFileSystemStorage and keeped overided code only in init |
according to documentation TenantFileSystemStorage() will create a separate directory for each schema_name under MEDIA_ROOT. This is not currently the case! My speculation is that this is broken because the subdir prefix is configured only once in the constructor of TenantFileSystemStorage() and then cached. Thus we end-up using the schema_name of the first tenant which happens to save files. The fact that TenantFileSystemStorageTestCase.test_format_string broke without changing anything else supports the above speculation. The reason is that the new test_files_are_saved_under_subdirectories_per_tenant() test method is executed *before* test_format_string()!
according to documentation TenantFileSystemStorage() will create a separate directory for each schema_name under MEDIA_ROOT. This is not currently the case! See: https://travis-ci.org/tomturner/djago-tenants/jobs/513242830 My speculation is that this is broken because the subdir prefix is configured only once in the constructor of TenantFileSystemStorage() and then cached. Thus we end-up using the schema_name of the first tenant which happens to save files. The fact that TenantFileSystemStorageTestCase.test_format_string broke without changing anything else supports the above speculation. The reason is that the new test_files_are_saved_under_subdirectories_per_tenant() test method is executed *before* test_format_string()!
…tenants#249 because the both depend on the current schema_name they can't be cached, nor can be initialized only once in the constructor. An important reason is that the `default_storage` class in Django is initialized only once! Also includes a small performance improvement which prevents the raising and catching of exceptions when MULTITENANT_RELATIVE_MEDIA_ROOT is not specified at all! Note: looks like this changed in django-tenants#198 but looking at the PR doesn't reveal why that is. There is a discussion about static file finders and template loaders but nothing for TenantFileSystemStorage().
|
Should these changes perhaps also apply to |
|
@jcass77 maybe but I don't use that storage nor have the capacity to figure out how not to break existing behavior for people who are using the other storage classes. |
|
@atodorov maybe write a alternative BetterTenantFileSystemStorage class and pull it :) ? |
|
I can confirm that this also applies to |
|
Linking in related issue #248. |
|
Here is the MR for this issue (the static files part): #255 |
|
@atodorov Can you fix your unit tests in #252 so @tomturner can review it, and also review #255 and merge them if they look fine? |
|
No, because I don't know how! As I've said in #252 (comment) using the standard executor everything works and I've also verified that manually but I have no idea why the multiprocessing executor fails and I am asking for help to debug or hints how to figure out what the problem is. You are doing work in the same space so maybe you can figure it out. Without more hints I'm kind of stuck on that multiprocessing test and not sure why Travis fails. |
|
@atodorov ok, let me see if I can get around this. Do you mind if I hijack your MR? |
|
go ahead, we can always cherry-pick, rebase or whatever. |
according to documentation TenantFileSystemStorage() will create a separate directory for each schema_name under MEDIA_ROOT. This is not currently the case! See: https://travis-ci.org/tomturner/djago-tenants/jobs/513242830 My speculation is that this is broken because the subdir prefix is configured only once in the constructor of TenantFileSystemStorage() and then cached. Thus we end-up using the schema_name of the first tenant which happens to save files. The fact that TenantFileSystemStorageTestCase.test_format_string broke without changing anything else supports the above speculation. The reason is that the new test_files_are_saved_under_subdirectories_per_tenant() test method is executed *before* test_format_string()!
…tenants#249 because the both depend on the current schema_name they can't be cached, nor can be initialized only once in the constructor. An important reason is that the `default_storage` class in Django is initialized only once! Also includes a small performance improvement which prevents the raising and catching of exceptions when MULTITENANT_RELATIVE_MEDIA_ROOT is not specified at all! Note: looks like this changed in django-tenants#198 but looking at the PR doesn't reveal why that is. There is a discussion about static file finders and template loaders but nothing for TenantFileSystemStorage().
|
@alexandernst and @atodorov thank you for all your hard work |
it's pretty easy to view the problem
just launch:
and in the shell:
the output will be:
The text was updated successfully, but these errors were encountered: