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

Getting ConnectionError in tox environment from call to check_bucket_existence when using static files storage #36

Open
alexolivas opened this issue Sep 20, 2022 · 2 comments

Comments

@alexolivas
Copy link

alexolivas commented Sep 20, 2022

Note: I'm only running into this issue in my CI/CD pipeline, because I am using tox to create an isolated python 3.8 environment to run all my django/python unit tests.

In my settings.py I set STATIC_FILES_STORAGE to use MinioBackendStatic to serve all my static files from my minio server, because of this I get a ConnectionError when the tox environment starts up, this library makes a call to check the buckets exist at django_minio_backend/apps.py#L32.

        # Validate static storage and default storage configurations
        staticfiles_storage: str = get_setting('STATICFILES_STORAGE')
        if staticfiles_storage.endswith(MinioBackendStatic.__name__):
            mbs = MinioBackendStatic()
            mbs.check_bucket_existence()

Screenshot of stacktrace on startup

Screen Shot 2022-09-20 at 10 24 50 AM

Because this is an isolated python environment specifically used for testing, it can't really reach out to ensure any buckets exist. By looking at the source code, it doesn't seem like there's a way to prevent the call to mbs.check_bucket_existence() unless I don't set STATICFILES_STORAGE = "django_minio_backend.models.MinioBackendStatic" in my settings.py file whenever I'm running this tox environment, which is not an elegant solution IMO.

I'm basically looking for any guidance on how to get around this a bit more elegantly. I'm also open to opening up a pull request to introduce an additional (or reuse MINIO_CONSISTENCY_CHECK_ON_START) configuration setting in that line of code to not check for bucket existence e.g.

There are 2 places where this needs to be done, in apps.py:

        consistency_check_on_start = get_setting('MINIO_CONSISTENCY_CHECK_ON_START', False)

        # other code goes here...

        # Validate static storage and default storage configurations
        staticfiles_storage: str = get_setting('STATICFILES_STORAGE')
        if staticfiles_storage.endswith(MinioBackendStatic.__name__):
            mbs = MinioBackendStatic()
            # This is my proposed change - this entire line could also be removed entirely
            # since the init method of MinioBackendStatic already checks for bucket existence
            if consistency_check_on_start:
                    mbs.check_bucket_existence()

and in models.py

@deconstructible
class MinioBackendStatic(MinioBackend):
    """
    MinIO-compatible Django custom storage system for Django static files.
    The used bucket can be configured in settings.py through `MINIO_STATIC_FILES_BUCKET`
    :arg *args: Should not be used for static files. It's here for compatibility only
    :arg **kwargs: Should not be used for static files. It's here for compatibility only
    """
    def __init__(self, *args, **kwargs):
        super().__init__(self.MINIO_STATIC_FILES_BUCKET, *args, **kwargs)

        consistency_check_on_start = get_setting('MINIO_CONSISTENCY_CHECK_ON_START', False)
        if consistency_check_on_start: # This is my proposed change
                self.check_bucket_existence()  # make sure the `MINIO_STATIC_FILES_BUCKET` exists
        self.set_bucket_to_public()  # the static files bucket must be publicly available
@theriverman
Copy link
Owner

I see your problem and I think your proposal to reuse MINIO_CONSISTENCY_CHECK_ON_START is a good call. I would like to avoid introducing yet another configuration parameter.

You're right about the proposed changes in those two places, but in the case of MinioBackendStatic, the last line calling self.set_bucket_to_public() should be indented into the if branch as well. Otherwise it may raise an Exception while trying to manipulate a non-existing bucket. At least that's what I suspect to happen.

Please feel free to open a PR, and I'll check it out ASAP.

@alexolivas
Copy link
Author

FYI I've not forgotten about this, I will open up a PR as soon as I free up a bit from work. I calculate sometime around mid next week.

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

No branches or pull requests

2 participants