-
Notifications
You must be signed in to change notification settings - Fork 101
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
Sometimes it's usefull to watch several simultaneous uploads in one request. Especially for a big number of such uploads. #30
base: master
Are you sure you want to change the base?
Conversation
How would I call this in a form and what javascript would I use to get multiple status for a particular unique ID? Do I have to give each upload a unique ID? Do you have an example of the config file you used? This looks exciting if I could get it working!!! |
Nope, you perform several uploads simultaneously when each has unique ID.
Yes.
Nothing special http {
upload_progress proxied 1m;
server {
listen 127.0.0.1:80;
location /progress {
report_uploads proxied;
upload_progress_json_multiple_output;
}
location ~ /upload/.* {
access_log /var/log/nginx/access.log main;
proxy_pass http://file-backend;
proxy_redirect off;
proxy_set_header Host $host;
proxy_set_header X-Real-IP $remote_addr;
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
proxy_intercept_errors on;
client_max_body_size 0;
client_body_buffer_size 512k;
client_body_temp_path /tmp/nginx/client_body_temp;
proxy_connect_timeout 120s;
proxy_send_timeout 120s;
proxy_read_timeout 120s;
proxy_buffer_size 256k;
proxy_buffers 128 4k;
proxy_busy_buffers_size 256k;
proxy_temp_file_write_size 256k;
track_uploads proxied 30s;
}
}
To get upload status you knock /progress with header X-ProgressMultiple-ID set to And you will get JSON array
Default pattern is :
|
@devgs Having a problem compiling your patch - keep getting build errors when running *make |
@thomascrown Maybe you are trying to compile different version of source code? |
@devgs Yup - it was my bad - no idea what file I was trying to use - i just replaced the .c file with your changed version. It seams to be working - thanks for the help so far - I really appreciated it! Now I have other problems: Do you have a working example of a form with javascript that has multiple uploads working? My config for Nginx server level is as follows: location @frontcontroller { location = /progress { location /upload { I cannot thank you enough for your help!!! |
@thomascrown That's me (devgs). Sorry for posting from different account. From what i read in your doc: upload_pass will not work. It says 'starting', because when there is no such ID found, you return such status. I hope that will help. So far i can't provide example because i am back-end developer and have poor knowledge of JS :( But soon we are going to use this in production and if you wish, I can give link you our website so you can test it with firebug. |
@devgs @SigSegFault Thanks for the help - I do have the front end working - and the setup that I pasted above works when I am using the non patched version of progress upload... In any case - I created a wrapper that accept dropped files, and that passes each file to the upload processor with a unique ID for each... it does the trick and Im getting accurate load times. Once again - many thanks for the help! |
Happy to help ;) Thank you. |
I know this is a long time with no answers on this PR, but can you please squash those commit in a more coherent patch set. Oh, and also move out the graphite commit in its own PR out of this one would be really good. Also it would be great to support the multiple ids with the exact same API as the single one (ie send the same headers, but detect when more than one id is sent and thus return a json array instead of a single json thing). Thanks, |
Signed-off-by: Vadim A. Misbakh-Soloviov <git@mva.name>
Signed-off-by: Vadim A. Misbakh-Soloviov <git@mva.name>
@masterzen While looking into the open PRs for this repo when I was updating the modules for my local nginx build today, I also did a cleanup of this PR. You can find the result of this work in my repo on branch My branch does not include any changes to support multiple IDs via the same API as you suggested in your most recent comment here either, but I think this should be doable with little extra effort. If you want I can take a look. If both @masterzen and @devgs agree, I can split my branch into three in order to supersede this PR. Please note, that my branch contains a major source code formatting patch in front of the actual work as the current master in this repository is quite inconsistent and the used formatting was all over the place. Using a more consistent formatting makes porting patches much more workable as there's a defined way to do things™ … This formatting is not fully perfect either, but a minimum step I deem necessary to simplify the further maintenance of the code base. Speaking of maintenance: @masterzen, would you agree to place this repo into a GH organization account and share (commit) access? The other option would be for me to hard-fork this repo in the long-run when integrating PRs that aren't yet merged here. This both duplicates work unnecessarily and also may cause confusion where active development takes place if there are multiple diverging repositories with ongoing changes. |
Would it be possible to open a PR here? I'm still not convinced the UDP extension is something that needs to be part of that project :)
I would say that we don't necessarily need it, but it's more a nice to have. If you want to spend a bit of time providing this, feel free to do so.
Yes, I do agree. You kept @devgs contributions, so I think you can do this even if we never get an answer from them.
Agreed. What did you use to format the code?
As you have noticed I'm not actively developing this project anymore as I don't need it anymore. Moving to an organization is one way of doing it, but I don't ever want to finish in a situation like the XZ project :( Can we first have this PR merged, and if you have more to merge let me know and I'll review merge accordingly. Then we can revisit the organization creation. |
Sure, just wanted to confirm with @devgs first, as it's basically their code.
Me neither. I'd also rather like to drop it and will do so for the PR. (@devgs : There will be a branch with the rebased changes for the UDP stuff too; I'll just wont update it further).
While looking through the source I noticed quite a few places that seem very repeated to me. And having just one API implemented is for sure a way to reduce code duplication (aside from some refactoring I'll try to get in later).
Will likely do some minor Git magic too to make ensure proper attribution.
The formatting was done by hand using the code style I usually follow. It's roughly based on the one from htop (indentation kept at 4 spaces ;-)), but as that one was mostly a first step I tried to keep it at a minimum. The htop guide also mostly gives a good guidance, instead of being strict. You can somewhat use
Understandable. Unfortunately that's always a risk with (unpaid) volunteer work. I'm running a (custom built) nginx on my infra with this and a few other modules added. Thus seeing these modules properly maintained is quite important.
Sure. Just wanted to bring this up as an option. No need to rush. |
@BenBE Go for it. |
Sorry if it looks hacky for you.