hackforla / peopledepot

A project to setup a datastore for people and projects at HackforLA. The link below takes you to the code documentation
https://hackforla.github.io/peopledepot/
GNU General Public License v2.0
5 stars 26 forks source link

Store project logos in django #91

Open fyliu opened 1 year ago

fyliu commented 1 year ago

Dependency

Overview

Suggestion from a PR review. The project image_logo field is a URL, but would be better managed by django.

Action Items

Discussion Items

Resources/Instructions

Should the image fields be ImageFields so Django can manage them? You would need to sort out how you will be storing the images - probably an S3 bucket for production but a local media volume for dev. You can use django-storages and a different MEDIA_URL configurations for dev and prod.

_Originally posted by @cnk in https://github.com/hackforla/peopledepot/pull/84#discussion_r985380281_

ExperimentsInHonesty commented 1 year ago

I would rather store the images in a GitHub repo and then use the URL it creates. Reasons:

Example:

I am open to understanding why it would be better to let Django store it in an S3 bucket, if there is a reason. Please detail

cnk commented 1 year ago

My reasoning for suggesting ImageFields rather than URLFields for images was in part because ImageFields would allow you to more easily keep a copy of the image rather than referencing it in a location H4LA does not control. A repository for the images might be OK - depending on the file sizes and the total size of the repository. But I don't know of a django package for storing user-uploaded images in S3. I would expect it would be possible to find or write one.

ExperimentsInHonesty commented 1 year ago

@cnk 1 - we are not worried about the size of the images in the repo. It would be no larger than what is currently stored in the hack for la website repo, which works fine 2 - it would be nice if we could use Django to authenticate that people are uploading images of the correct dimensions and size, which would be a valid reason for having an image field rather than a URL field, unless validating the image dimensions and size is possible on URLs too?

Thoughts?

cnk commented 1 year ago

1 - we are not worried about the size of the images in the repo. It would be no larger than what is currently stored in the hack for la website repo, which works fine

Sounds like you expect similar data so this is probably OK to store in Git - especially with large file support. I suspect PeopleDepot will have more images/files because it is easier to upload from a JS frontend than it is to add the same file via a pull request but as long as the desired files are similar, that shouldn't be too big a deal.

2 - it would be nice if we could use Django to authenticate that people are uploading images of the correct dimensions and size, which would be a valid reason for having an image field rather than a URL field, unless validating the image dimensions and size is possible on URLs too?

Validation is more certain if you maintain control of the image via the same code that is doing the validation. If we use an image field, we can get Django to handle image upload and validation. One could do similarly by downloading the image from the url and then validating it. But I have less confidence that the image url will continue to point to the validated image file if that file isn't managed by django itself.

I do, however, urge you to consider the fundamental tradeoff in storage options - money vs time. If you use GitHub to hold your images, there is no hosting fee - but I am not seeing a django package that uses git as the file storage backend. If you use an S3 bucket or other backend supported by django-storages, file storage can be configured in an hour or so. Unless someone else finds a 'store django media files in git' package, it is going to take a lot longer than that to write the storage code.

ExperimentsInHonesty commented 1 year ago

@cnk @fyliu ok, then we will use Django and aws s3 bucket

fyliu commented 11 months ago

Moving this to Questions. @bonniewolfe I vaguely remember discussing as a team and deciding that we're going to store the logo images in github and their urls in django for now? If so, we should close this issue or move it to the Ice Box if we want to reconsider it later, like after a certain deployment milestone.

ExperimentsInHonesty commented 3 months ago

We 100% do not want to pay for image transfers from AWS to our websites, if we can store the images in a repository and have the website access them on the build, we should.