-
Notifications
You must be signed in to change notification settings - Fork 73
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
Added temporary storage via settings flag #39
base: master
Are you sure you want to change the base?
Conversation
… store chunks in OS temporary storage
@juliomalegria Please have a look at my PR and consider merging it. In combination with django-storages 1.6.6 and boto3 to works for Amazon S3. |
@juliomalegria Any chance of getting this in the master branch and release a new version? |
No update so far? |
# Conflicts: # README.rst # chunked_upload/settings.py
@jerinpetergeorge I fixed the merge conflicts. Whats your opinion on this topic? |
I have tried the new changes in my local machine. Unfortunately, it's not uploaded to S3 Step to reproducePart-1
Part-2 ( checks whether default s3 media/file upload working or not)
Part-3 ( checking
|
@GitRon |
@jerinpetergeorge I'm sorry but I cant follow you. We are using boto3 as a connector and its file storage class. So you dont have to worry about the upload at all. |
Oops 😣 Anyway, let me ask you some doubt regarding this, that may help you to understand what I am trying to say
OR, Could you provide a minimal example that demonstrates the S3 upload? |
Ok, I think we are talking not about the same thing here 😅 My changes are not about ENABLING cloud storag uploads. Its just prerequisite. If you are writing to a file storage like a hard drive, it does not matter where you park the chunks. On an object storage it does because you cannot write to an object storage. It only accepts full files as an object. I gave it a second thought and maybe we should always - without the settings flag - write the chunks to the temp storage. I think this is the main reason why there is a temp-dir, right? To contain temporary data. What do you think? |
Storing the chunks to a temporary location and moving it to the target location after completion is a good idea. I think we should implement this first before the S3 thing. |
Perfect, I'll change my PR then. |
@jerinpetergeorge Ok, the behavior is now as follows: If you define no storage, it takes my temporary storage. But you can override it with your settings var if you like. I like this solution, good that we talked about it 👍 |
Great! but this is only half of the solution. We should move the |
@jerinpetergeorge Sound good. But doesn't it already work like this? I just collect the chunks in temp storage. |
@jerinpetergeorge Somebody asked for my fix on this issue. Whats your status? I pointed out - after thinging about the things that happended - that me might want to avoid renaming variables etc. To maybe make it stable and compatible again? What do you think? 😃 |
Sorry for the utter delay from my side. Thanks for your effort @GitRon and please do let me give a couple of days to recollect the project context ( 😲 ) and review the PR as I am completely slipped away from this. |
@GitRon Thanks for the PR. But, It is hard for me to review the PR since I have slipped away from the context, as I already said. Also, I don't think I can make it to work in the near future. So, I would recommend you to ask @juliomalegria to make you a collaborator of this repo or fork this project and maintain it yourself. |
@jerinpetergeorge @juliomalegria Sure, if you give me permission I'd fix the bug people were complaining about. Would you be able to deploy a new version at the time? |
I won't be able to do that 😞 You can contact @juliomalegria via his personal mail ( FYI: There is a place called jazzband, and I think we can move this project there and which will help to get more exposure. There are some guidelines to make it work, jazzband guidelines, please have a look. |
@jerinpetergeorge I know, you were just in CC. But good idea with the personal email address! Thx! And good idea for jazzband as well! |
@jerinpetergeorge I wrote him. We'll see what happens. |
Any luck with this, I echo the use case that chunks being uploaded to S3 directly would be preferential. |
@cr0mbly I am not sure if this is possible and wanted. S3 is not a file system but an object storage. So you'd expect the full file as an object, not some bunch of chunks. Furthermore: What happens if something goes sideways during the upload? Then you have to clean up some leftover chunks? IMHO the local temp approach is the cleaner solution. 😃 |
Yep thanks for this. I just put some work in on my own to integrate S3 as the default storage and came up against the fact that we can't append file to an S3 object. This is fine for small files as you can just keep updating the file during the backend but when you get larger files loading and downloading the full object into memory becomes less and less preformative. Just for anyone else looking at this with autoscaling setup we left it as tempstorage and used EFS to maintain persistence of the filesystem during chunk upload. |
If you use AWS S3 the default storage is not working. The chunks cannot be created and updated.
I added the possibility to activate cross-platform temporary storage. I cannot use
CHUNKED_UPLOAD_STORAGE_CLASS
because I need to pass thelocation
paramter to the storage.This is not working.
I decided not to change the way
CHUNKED_UPLOAD_STORAGE_CLASS
works for better compatibility.