Review Request: New KIO::http_post and KIO::StoredHttpPost APIs that accept a QIODevice as input...
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/100516/ --- Review request for kdelibs. Changes --- Did some basic testing of the changes in kio_http using khtml. The repost scenario is still untested after these new changes. Summary --- The attached patch is the first portion a set of patches to make uploading data through HTTP more efficient without affecting the existing implementation. Right now the amount of memory consumed when uploading large files through http or webdav is really not acceptable because only a QByteArray based API is available. That means if you want to upload a file of say 50 or 100 MB to a server, then you have to read the entire thing first before you can call KIO::http_post! This addresses bug 34578. http://bugs.kde.org/show_bug.cgi?id=34578 Diffs - kio/kio/accessmanager.cpp 722203f kio/kio/global.h 2aa8ebb kio/kio/global.cpp b658d91 kio/kio/job.h 632dfc8 kio/kio/job.cpp 7d4a849 kio/kio/job_p.h daac895 kio/kio/jobclasses.h e9bd191 kioslave/http/http.h be08d7b kioslave/http/http.cpp 187e3c7 Diff: http://git.reviewboard.kde.org/r/100516/diff Testing (updated) --- ** Used by changing kdewebkit to use the new API. ** Tested some with khtml to ensure the kio_http changes did not cause any regression. Thanks, Dawit
Re: Review Request: New KIO::http_post and KIO::StoredHttpPost APIs that accept a QIODevice as input...
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/100516/#review1305 --- Ship it! Looks good, but... kio/kio/job.cpp http://git.reviewboard.kde.org/r/100516/#comment1086 Shouldn't there be a kio_http patch to go along with this? Otherwise the post data size is not used. Or was that just for the future? - David On Feb. 7, 2011, 6:46 p.m., Dawit Alemayehu wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/100516/ --- (Updated Feb. 7, 2011, 6:46 p.m.) Review request for kdelibs. Summary --- The attached patch is the first portion a set of patches to make uploading data through HTTP more efficient without affecting the existing implementation. Right now the amount of memory consumed when uploading large files through http or webdav is really not acceptable because only a QByteArray based API is available. That means if you want to upload a file of say 50 or 100 MB to a server, then you have to read the entire thing first before you can call KIO::http_post! This addresses bug 34578. http://bugs.kde.org/show_bug.cgi?id=34578 Diffs - kio/kio/job.h 632dfc8 kio/kio/job.cpp 7d4a849 kio/kio/job_p.h daac895 kio/kio/jobclasses.h e9bd191 Diff: http://git.reviewboard.kde.org/r/100516/diff Testing --- Used by changing kdewebkit to use the new API. Thanks, Dawit
Re: Review Request: New KIO::http_post and KIO::StoredHttpPost APIs that accept a QIODevice as input...
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/100516/ --- (Updated Feb. 7, 2011, 6:46 p.m.) Review request for kdelibs. Changes --- * Added size parameter to the new POST APIs based on discussion. * Changed both the new and old POST APIs to always send content size information to the ioslaves. Summary --- The attached patch is the first portion a set of patches to make uploading data through HTTP more efficient without affecting the existing implementation. Right now the amount of memory consumed when uploading large files through http or webdav is really not acceptable because only a QByteArray based API is available. That means if you want to upload a file of say 50 or 100 MB to a server, then you have to read the entire thing first before you can call KIO::http_post! This addresses bug 34578. http://bugs.kde.org/show_bug.cgi?id=34578 Diffs (updated) - kio/kio/job.h 632dfc8 kio/kio/job.cpp 7d4a849 kio/kio/job_p.h daac895 kio/kio/jobclasses.h e9bd191 Diff: http://git.reviewboard.kde.org/r/100516/diff Testing --- Used by changing kdewebkit to use the new API. Thanks, Dawit
Re: Review Request: New KIO::http_post and KIO::StoredHttpPost APIs that accept a QIODevice as input...
Is QIODevice the best idea to use as source? Since we are talking KIO, I believe we can espect the user of KIO::http_post to be using KIO and not Qt IO. So would it instead be possible to make the source a KIO job or KUrl? Regards `Allan On Wednesday 02 February 2011, Dawit Alemayehu wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/100516/ --- Review request for kdelibs. Summary --- The attached patch is the first portion a set of patches to make uploading data through HTTP more efficient without affecting the existing implementation. Right now the amount of memory consumed when uploading large files through http or webdav is really not acceptable because only a QByteArray based API is available. That means if you want to upload a file of say 50 or 100 MB to a server, then you have to read the entire thing first before you can call KIO::http_post! This addresses bug 34578. http://bugs.kde.org/show_bug.cgi?id=34578 Diffs - kio/kio/jobclasses.h e9bd191 kio/kio/job_p.h daac895 kio/kio/job.cpp 7d4a849 kio/kio/job.h 632dfc8 Diff: http://git.reviewboard.kde.org/r/100516/diff Testing --- Used by changing kdewebkit to use the new API. Thanks, Dawit
Re: Review Request: New KIO::http_post and KIO::StoredHttpPost APIs that accept a QIODevice as input...
Ahh... I think you misunderstood the purpose of the patch or rather the title of this review. The new APIs simply overload the existing http post APIs such that the data you are going to post is sent through a QIODevice (QFile or QBuffer) rather than a QByteArray. To be clear here is the new overloaded API: KIO_EXPORT TransferJob *http_post( const KUrl url, QIODevice* device, JobFlags flags = DefaultFlags ); compared to the existing one KIO_EXPORT TransferJob *http_post( const KUrl url, const QByteArray _staticData, JobFlags flags = DefaultFlags ); The flaws of the old API are very obvious and have been an issue since ages. We got a 9 year old bug report about it. :( Passing the data to be uploaded as a QByteArray is fine for small post data like this repose I am writing in a web mail client. However, the memory usage it will incur when normal users attempt to upload today's multimedia objects (music, videos, and even some image files) is really not acceptable. That is what the new API is trying to address. Hope that clarifies it... On Fri, Feb 4, 2011 at 7:12 AM, Allan Sandfeld Jensen k...@carewolf.com wrote: Is QIODevice the best idea to use as source? Since we are talking KIO, I believe we can espect the user of KIO::http_post to be using KIO and not Qt IO. So would it instead be possible to make the source a KIO job or KUrl? Regards `Allan On Wednesday 02 February 2011, Dawit Alemayehu wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/100516/ --- Review request for kdelibs. Summary --- The attached patch is the first portion a set of patches to make uploading data through HTTP more efficient without affecting the existing implementation. Right now the amount of memory consumed when uploading large files through http or webdav is really not acceptable because only a QByteArray based API is available. That means if you want to upload a file of say 50 or 100 MB to a server, then you have to read the entire thing first before you can call KIO::http_post! This addresses bug 34578. http://bugs.kde.org/show_bug.cgi?id=34578 Diffs - kio/kio/jobclasses.h e9bd191 kio/kio/job_p.h daac895 kio/kio/job.cpp 7d4a849 kio/kio/job.h 632dfc8 Diff: http://git.reviewboard.kde.org/r/100516/diff Testing --- Used by changing kdewebkit to use the new API. Thanks, Dawit
Re: Review Request: New KIO::http_post and KIO::StoredHttpPost APIs that accept a QIODevice as input...
On Friday 04 February 2011, Allan Sandfeld Jensen wrote: Is QIODevice the best idea to use as source? I believe we can espect the user of KIO::http_post to be using KIO and not Qt IO I like QIODevice actually, for reading stuff on demand. I use it everywhere in KArchive (KZip, KTar...) and KFilterDev. QIODevice is a pull solution - you can ask for 1MB of data now. Well, at least with buffers and files, not necessarily with sockets :-) The good thing with QIODevice is that it works on top of both memory (QBuffer) and files (QFile), so in kde5 we could even remove the QByteArray overload. And if you want to send bzip2 compressed data you could even insert a KFilterDev to do it on the fly. KIO::Job on the other hand is a push solution - it will tell you when it has something. Well in this case both would work I guess, slotDataReqFromDevice could just send whatever is available, using an intermediate storage for what the underlying get job is emitting. But the only benefit would be being able to upload remote data, not sure if we have a use case for that. -- David Faure, fa...@kde.org, http://www.davidfaure.fr Sponsored by Nokia to work on KDE, incl. Konqueror (http://www.konqueror.org).
Re: Review Request: New KIO::http_post and KIO::StoredHttpPost APIs that accept a QIODevice as input...
Em sexta-feira, 4 de fevereiro de 2011, às 16:52:54, Allan Sandfeld Jensen escreveu: Or to put another way; PUT takes a KUrl to send to and gets the data it sends from a slot. POST is essentially just a PUT with return data, so I would find it most natural if POST mirrored PUT in how it sends data just like it mirrors GET in how it receives it. A PUT-from-GET operation is called copy and we already have that operation: KIO::file_copy. A streaming POST is not a common case, though, because POSTs most often require a form-like transport or, in the case of SOAP, XML. -- Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org Senior Product Manager - Nokia, Qt Development Frameworks PGP/GPG: 0x6EF45358; fingerprint: E067 918B B660 DBD1 105C 966C 33F5 F005 6EF4 5358 signature.asc Description: This is a digitally signed message part.
Review Request: New KIO::http_post and KIO::StoredHttpPost APIs that accept a QIODevice as input...
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/100516/ --- Review request for kdelibs. Summary --- The attached patch is the first portion a set of patches to make uploading data through HTTP more efficient without affecting the existing implementation. Right now the amount of memory consumed when uploading large files through http or webdav is really not acceptable because only a QByteArray based API is available. That means if you want to upload a file of say 50 or 100 MB to a server, then you have to read the entire thing first before you can call KIO::http_post! This addresses bug 34578. http://bugs.kde.org/show_bug.cgi?id=34578 Diffs - kio/kio/jobclasses.h e9bd191 kio/kio/job_p.h daac895 kio/kio/job.cpp 7d4a849 kio/kio/job.h 632dfc8 Diff: http://git.reviewboard.kde.org/r/100516/diff Testing --- Used by changing kdewebkit to use the new API. Thanks, Dawit
