Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document how to change file system storage default file name #5069

Closed
banagale opened this issue Mar 26, 2021 · 9 comments · Fixed by #5152
Closed

Document how to change file system storage default file name #5069

banagale opened this issue Mar 26, 2021 · 9 comments · Fixed by #5152

Comments

@banagale
Copy link
Contributor

banagale commented Mar 26, 2021

Summary

The media pipeline, supported storage, file system storage section of the documentation begins by explaining that files are stored using SHA1 hash of their URLs.

This is a good default choice, but the documentation makes no mention of the MediaPipeline.file_path() method.

Presuming this is indeed the best way to get custom file names out of a file or images pipeline, this issue is to request a call out for setting a custom file name by overriding this method:

    def file_path(self, request, response=None, info=None, *, item=None):
        image_guid = hashlib.sha1(to_bytes(request.url)).hexdigest()
        return f'full/{image_guid}.jpg'

Motivation

It appears it used to be more tricky to customize file names.

Calling out this method may save people time.

Describe alternatives you've considered

Leaving it as is or perhaps explaining why SHA1 was chosen over any other default file naming scheme.

@aadityasinha-dotcom
Copy link

Can I work on this issue?

@banagale
Copy link
Contributor Author

Fine with me.

@aadityasinha-dotcom
Copy link

Can you explain me about this issue?

@banagale
Copy link
Contributor Author

Please re-read the issue and if you have a specific question that is not addressed let me know.

@aadityasinha-dotcom
Copy link

I have to make the file_path() method in the media.pipeline?

@banagale
Copy link
Contributor Author

@aadityasinha-dotcom I'm not sure I understand what you're looking for, Aaditya. This issue is related to the tool's documentation. 

I've posted it here for feedback on if there is a better way to set this filename. 

If not, and there are no objections to a PR adding a note about this behavior, I'll do it myself.

@Gallaecio
Copy link
Member

This is technically already documented.

But maybe we could improve discoverability by adding a section to that page about customizing the output file path. The section should probably just say that you can do that by customizing the middleware and overriding file_path, with links to both sections below.

@banagale
Copy link
Contributor Author

banagale commented Apr 5, 2021

@Gallaecio I think that this would be sufficient. I had not seen the class definition, but discovered it looking through the code.

Let me know if you would you be open to a PR for this change, and I'll add something reasonable.

@Gallaecio
Copy link
Member

@banagale That would be great, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants