Git Product home page Git Product logo

Comments (6)

laymonage avatar laymonage commented on May 30, 2024 1

Thanks for the quick response @theriverman!
I haven't got the time to test it, but I believe it's a step in the right direction 👍

from django-minio-backend.

rcy17 avatar rcy17 commented on May 30, 2024

I'm not sure if I really got it, but currently I just modify djabgo_minio_backend.models like this:

    def _open(self, object_name, mode=None, bucket_name=None,  request_headers=None, sse=None):
        if bucket_name is None:
            bucket_name = self._BUCKET
        return self.client.get_object(bucket_name, object_name, request_headers, sse)

Now it works.

from django-minio-backend.

theriverman avatar theriverman commented on May 30, 2024

Thanks for your detailed report!

I've just tested it in my local environment after bumping Django to 3.x and the underlying minio-sdk to the latest version and all seems to work.
Next thing to see if it's related to the used Minio version itself (I'm locally running version 2019-09-18).

Could you provide your get_image_path implementation and/or tell me its typical return value and type, please?

Modifying _open() shouldn't be needed, because the Exception is raised in the SDK itself.

Edit: Bumped Minio to latest version, still OK!

Thanks for reporting the issue with is_minio_available, I've updated the README.

from django-minio-backend.

theriverman avatar theriverman commented on May 30, 2024

Please reopen this issue if you're still facing this error.

from django-minio-backend.

laymonage avatar laymonage commented on May 30, 2024

Hi there, I think this is still an issue.

I didn't encounter the issue directly when fetching the object, but when I try to access the file using instance.image.open(), I got the same error.

What was the file name that you used for the test, @theriverman? I suppose if the whole file path in the bucket does not violate the bucket naming rules, it won't be an issue. However, try uploading the file under a directory, such as mydir/myfile.png.

As @rcy17 has pointed out, your _open method signature is wrong. In order to write a custom storage system, you need to implement _open(name, mode='rb') as specified in the Django docs. With your current _open method signature, the file name gets passed as the bucket_name.

I don't see any reason why you would add bucket_name as a parameter (since there's no point trying to access the file from a different bucket), so I think the following fix is enough:

    def _open(self, object_name, request_headers=None, sse=None):
        return self.client.get_object(self._BUCKET_NAME, object_name, request_headers, sse)

    def open(self, object_name, request_headers=None, sse=None):
        return self._open(object_name, request_headers, sse)

If you don't want to override open, then you'll have to add mode=None before request_headers so that the default value of mode='rb' doesn't mistakenly get passed as request_headers (see https://github.com/django/django/blob/master/django/core/files/storage.py#L34)

from django-minio-backend.

theriverman avatar theriverman commented on May 30, 2024

Hi @laymonage,

You're right, this was still an issue. I've read the trackback superficially copied by @rcy17 .
I have modified the _open method in the following way:

    def _open(self, object_name, mode='rb', **kwargs):
        """
        Implements the Storage._open(name,mode='rb') method
        :param name (str): object_name [path to file excluding bucket name which is implied]
        :kwargs (dict): pass on to the underlying minIO client's get_object() method
        """
        resp: urllib3.response.HTTPResponse = urllib3.response.HTTPResponse()

        if mode != 'rb':
            raise ValueError('Files retrieved from minIO are read-only. Use save() method to override contents')
        try:
            resp = self.client.get_object(self._BUCKET_NAME, object_name, kwargs)
            file = File(file=io.BytesIO(resp.read()), name=object_name)
        finally:
            resp.close()
            resp.release_conn()
        return file

According to Django docs, the _open method shall return a File object, so I wrap the returned bytes into that.
Also, according to the minIO SDK, the connection pool created by self.client.get_object shall be closed and released, so I put all this into a try/finally block.

Please let me know your opinion on this approach. Does it satisfy the scenarios when instance.image.open() is called?

from django-minio-backend.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.