Open Forchapeatl opened 8 months ago
The problem now is that we have the decode("utf-8")
directly in the read()
method.
The decoding should not happens in the read method since we don't know for sure what is stored in the S3 bucket.
The best alternative is to let the developer handle the decoding him/herself, which mean the read()
method should return the actual binary form of the read data.
We need to add tests to make sure that when the data is not text, but a file or a image, the correct binary form is returned.
So, you propose to turn the S3ObjectDataNode
into a fully generic data node.
In addition, does it make sense to have an S3TextDataNode
that reads the object and decodes it directly, so the main use case remains simple for the developer? What do you think?
Just like we have an SQLDataNode
, which is completely generic, we also have an SQLTableDataNode
to make it simple to read a table (the main usage).
Store only text in a S3bucket is not really a popular practice. I would suggest keeping it generic for now, and we can expand the functionality in the future when it is required.
It does n't read on images , video and audio data . This is because we are reading
uft-8
encodings here which is fair :)removing
.decode('utf-8')
will mean thes3ObjectDataNode.read()
will have to be called ass3ObjectDataNode.read().decode('utf-8')
when trying to access text files.I don't think this
s3ObjectDataNode.read().decode('utf-8')
fuction call is the expected utility of the Datanode classread()
method. Besides, this is an overkill !I would like to propose that the write class method should enable one more property to the
S3ObjectDataNode
. This ACL - Permision management parameter. Adding a new property to the s3ObjectDataNode will solve this media object reading trouble. It will allow users to read s3 object media files by Url.With the new property set to
public-read
_( ACL = publicread ) the user will be able to access media files (image, vide , audio) files via url .This means the read method will handle ASCII data only and the user will be free to read Media files via url.
I will like to leave this open for discussion. I just didn't want to forget , we can transfer this to a new issue when this PR is handled.
Originally posted by @Forchapeatl in https://github.com/Avaiga/taipy/issues/585#issuecomment-1849066118