[Django] #26896: FileSystemStorage no longer accepts reverse_lazy as base_url
Django
--------------------------------------+-----------------------------------
Reporter: tpazderka | Owner: nobody
Type: Bug | Status: new
Component: File uploads/storage | Version: 1.8
Severity: Normal | Keywords: storage, reverse_lazy
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+-----------------------------------
Changes introduced with commit fb9d8f06 fixing #22717 causes
`FileSystemStorage` to evaluate the content of `base_url` uppon
initialization.
This causes issues if the `base_url` is defined as `reverse_lazy` as it is
resolved and can cause `ImportError`.
I think that the laziness of the `base_url` should be preserved. Either
the check for the trailing slash can me moved to path generation or the
`__init__` can be updated to expect `reverse_lazy` as `base_url`.
--
Ticket URL: <https://code.djangoproject.com/ticket/26896>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: tpazderka | Owner: nobody
Type: Bug | Status: new
uploads/storage |
Severity: Normal | Resolution:
Keywords: storage, | Triage Stage:
reverse_lazy | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Old description:
> Changes introduced with commit fb9d8f06 fixing #22717 causes
> `FileSystemStorage` to evaluate the content of `base_url` uppon
> initialization.
> This causes issues if the `base_url` is defined as `reverse_lazy` as it
> is resolved and can cause `ImportError`.
>
> I think that the laziness of the `base_url` should be preserved. Either
> the check for the trailing slash can me moved to path generation or the
> `__init__` can be updated to expect `reverse_lazy` as `base_url`.
New description:
Changes introduced with commit [fb9d8f06] fixing #22717 causes
`FileSystemStorage` to evaluate the content of `base_url` uppon
initialization.
This causes issues if the `base_url` is defined as `reverse_lazy` as it is
resolved and can cause `ImportError`.
I think that the laziness of the `base_url` should be preserved. Either
the check for the trailing slash can me moved to path generation or the
`__init__` can be updated to expect `reverse_lazy` as `base_url`.
--
Comment:
Could you provide a test for `tests/file_storage/tests.py` that
demonstrates your use case?
--
Ticket URL: <https://code.djangoproject.com/ticket/26896#comment:1>
Django
Reporter: tpazderka | Owner: nobody
Type: Bug | Status: new
uploads/storage |
Severity: Normal | Resolution:
Keywords: storage, | Triage Stage:
reverse_lazy | Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by tpazderka):
* Attachment "0001-Test-for-reverse_lazy.patch" added.
Patch for tests/file_storage/tests.py
Django
Reporter: tpazderka | Owner: nobody
Type: Bug | Status: new
uploads/storage |
Severity: Normal | Resolution:
Keywords: storage, | Triage Stage:
reverse_lazy | Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by tpazderka):
Not exactly our usecase as our failure is on `ImportError` which seems
hard to test, but I hope that this patch shows the issue we are having.
The basic idea is that `reverse_lazy` should remain lazy as long as
possible. Resolving the `base_url` on `init()` defeats the purpose of
using `reverse_lazy` there.
--
Ticket URL: <https://code.djangoproject.com/ticket/26896#comment:2>
Django
Reporter: tpazderka | Owner: nobody
Type: Bug | Status: new
uploads/storage |
Severity: Normal | Resolution:
Keywords: storage, | Triage Stage:
reverse_lazy | Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* cc: claudep (added)
Comment:
Should we skip that check if `base_url` is lazy or do you see some
alternative?
--
Ticket URL: <https://code.djangoproject.com/ticket/26896#comment:3>
Django
Reporter: tpazderka | Owner: nobody
Type: Bug | Status: new
uploads/storage |
Severity: Normal | Resolution:
Keywords: storage, | Triage Stage:
reverse_lazy | Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by claudep):
We should explore making `base_url` a descriptor, that might help delaying
the check.
--
Ticket URL: <https://code.djangoproject.com/ticket/26896#comment:4>
Django
Reporter: tpazderka | Owner: nobody
Type: Bug | Status: new
uploads/storage |
Severity: Normal | Resolution:
reverse_lazy |
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/26896#comment:5>
Django
Reporter: tpazderka | Owner: nobody
Type: Bug | Status: new
uploads/storage |
Severity: Normal | Resolution:
Keywords: storage, | Triage Stage: Accepted
reverse_lazy |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by tpazderka):
* has_patch: 0 => 1
Comment:
I have created a pull request https://github.com/django/django/pull/6989
which omitts the check if the `base_url` is not of `string_types`.
--
Ticket URL: <https://code.djangoproject.com/ticket/26896#comment:6>
Django
Reporter: tpazderka | Owner: nobody
Type: Bug | Status: closed
Component: File | Version: 1.8
uploads/storage |
Keywords: storage, | Triage Stage: Accepted
reverse_lazy |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"b820b6108a7d3f11ec18774d16d657f4f63fe9fa" b820b61]:
{{{
#!CommitTicketReference repository=""
revision="b820b6108a7d3f11ec18774d16d657f4f63fe9fa"
Fixed #26896 -- Allowed a lazy base_url for FileSystemStorage.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26896#comment:7>
Django
Reporter: tpazderka | Owner: nobody
Type: Bug | Status: closed
Component: File | Version: 1.8
uploads/storage |
Severity: Normal | Resolution: fixed
Keywords: storage, | Triage Stage: Accepted
reverse_lazy |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"d61dbc20cf135cde18bb45411fa46c44dc4c17c2" d61dbc2]:
{{{
#!CommitTicketReference repository=""
revision="d61dbc20cf135cde18bb45411fa46c44dc4c17c2"
[1.10.x] Fixed #26896 -- Allowed a lazy base_url for FileSystemStorage.
Backport of b820b6108a7d3f11ec18774d16d657f4f63fe9fa from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26896#comment:8>