search

theriverman / django-minio-backend

Minio Backend for Django
https://pypi.org/project/django-minio-backend/
MIT License
111 stars 22 forks source link

Fixed validate settings #31

Closed abhi1693 closed 2 years ago

abhi1693 commented 2 years ago

I had the MINIO_USE_HTTPS=False in my settings and the library failed to start as validate_settings requires USE_HTTPS as True which should not be enforced on the user.

I have removed it from mandatory settings and set it's default value as False

abhi1693 commented 2 years ago

@theriverman Can you please validate this?

theriverman commented 2 years ago

Hi, @abhi1693

Thanks for the PR, this is indeed a bug due to my poor solution. However, I'm not perfectly happy with this proposed change:

self.__MINIO_USE_HTTPS: bool = get_setting("MINIO_USE_HTTPS", False)

I haven't set default values for these settings parameters because I would like to enforce the user to explicitly configure it. We can't assume whether their used MinIO instance will be http or https, so the best option is keeping this responsibility in the user's hand.

I propose to go with something like this

mandatory_parameters = (self.__MINIO_ENDPOINT, self.__MINIO_ACCESS_KEY, self.__MINIO_SECRET_KEY)
if any([bool(x) is False for x in mandatory_parameters]) or (get_setting("MINIO_USE_HTTPS") is None):
    raise ConfigurationError(
        "A mandatory parameter (ENDPOINT, ACCESS_KEY, SECRET_KEY or USE_HTTP) hasn't been configured properly"
    )