Intended effect of returning None from ManifestFilesMixin.file_hash

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

Intended effect of returning None from ManifestFilesMixin.file_hash

Richard Campen
Hello

I am currently looking at implementing a custom file hashing function when using the ManifestFilesMixin class to provide white list based file hashing of static files.

In the process I have found what seems to be an unintended result of returning `None` from a custom `file_hash()` function when using the mixin. Looking at the original ticket that resulted in the refactoring of the file hashing into it's own function (for the explicit intention of enabling custom algorithms when using the mixin) there is no indication of intent for the current behaviour.

Essentially, when returning a string from a custom` file_hash()` implementation, the resulting file name is `<file_path>.<custom_hash>.<ext>` as expected, whereas returning `None` results in `<file_path>None.<ext>`

The fact that the string `None` appears in the file name seems undesirable, and the fact it occurs without a preceding `.` (inconsistent with the filename structure when a string is returned) suggests to me it is unintentional.

I have my own fix to this locally that leaves the file name untouched if `file_hash()` returns `None`. Before going through the process of starting a ticket etc. I was curious if anyone had any thoughts on the intent of injecting `None` into the file name. Bug or feature.

Cheers
Richard

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/050868a3-b941-4cd1-b4e4-7b1b6329d8b4%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Intended effect of returning None from ManifestFilesMixin.file_hash

Adam Johnson-2
That does seem like an unintentional bug, since the code is forming a string under the condition "if file_hash is not None:".

On Sun, 25 Nov 2018 at 03:26, Richard Campen <[hidden email]> wrote:
Hello

I am currently looking at implementing a custom file hashing function when using the ManifestFilesMixin class to provide white list based file hashing of static files.

In the process I have found what seems to be an unintended result of returning `None` from a custom `file_hash()` function when using the mixin. Looking at the original ticket that resulted in the refactoring of the file hashing into it's own function (for the explicit intention of enabling custom algorithms when using the mixin) there is no indication of intent for the current behaviour.

Essentially, when returning a string from a custom` file_hash()` implementation, the resulting file name is `<file_path>.<custom_hash>.<ext>` as expected, whereas returning `None` results in `<file_path>None.<ext>`

The fact that the string `None` appears in the file name seems undesirable, and the fact it occurs without a preceding `.` (inconsistent with the filename structure when a string is returned) suggests to me it is unintentional.

I have my own fix to this locally that leaves the file name untouched if `file_hash()` returns `None`. Before going through the process of starting a ticket etc. I was curious if anyone had any thoughts on the intent of injecting `None` into the file name. Bug or feature.

Cheers
Richard

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/050868a3-b941-4cd1-b4e4-7b1b6329d8b4%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


--
Adam

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAMyDDM2rAwHYy5KZw_t4Lrzz3zaAw1cwOvFoJMW%3DCPsw4U%3D6hQ%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.