Add asynchronous downloader that reuses connections to CCDB - #9992
Closed
TrifleMichael wants to merge 1 commit into
Closed
Add asynchronous downloader that reuses connections to CCDB#9992TrifleMichael wants to merge 1 commit into
TrifleMichael wants to merge 1 commit into
Conversation
TrifleMichael
requested review from
a team,
Barthelemy,
costing and
sawenzel
as code owners
October 4, 2022 15:32
TrifleMichael
force-pushed
the
AsynchronousDownloadsSquashed
branch
3 times, most recently
from
October 10, 2022 11:01
5e48bfa to
e7ad5bc
Compare
TrifleMichael
requested review from
a team,
afurs,
andreasmolander,
bazinski,
bovulpes,
davidrohr,
f3sch,
iouribelikov,
jotwinow,
kharlov,
knopers8,
marcovanleeuwen,
martenole,
mconcas,
noferini,
peressounko,
qgp,
rpezzi,
sgorbuno,
shahor02,
tdietel and
wiechula
as code owners
October 10, 2022 11:04
TrifleMichael
requested review from
ktf
and removed request for
afurs,
andreasmolander,
arvindkhuntia,
bovulpes,
chiarazampolli,
f3sch,
iouribelikov,
jokonig,
jotwinow,
kharlov,
knopers8,
marcovanleeuwen,
martenole,
mslupeck,
noferini,
peressounko,
rpezzi,
sgorbuno and
wiechula
November 17, 2022 10:42
Collaborator
|
@ktf bumping this thread not to loose track of it |
Collaborator
|
@ktf , did you have a chance at checking out the updated pr ? |
ktf
reviewed
Dec 5, 2022
|
|
||
| void CcdbApi::curlInit() | ||
| { | ||
| std::cout << "Creating downloader\n"; |
ktf
reviewed
Dec 5, 2022
| curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, write_data); | ||
| curlSetSSLOptions(curl); | ||
| res = curl_easy_perform(curl); | ||
| std::cout << "mUrl " << mUrl.data() << "\n"; |
ktf
reviewed
Dec 5, 2022
|
|
||
| size_t writeToString(void *contents, size_t size, size_t nmemb, std::string *dst) | ||
| { | ||
| char *conts = (char *)contents; |
Member
There was a problem hiding this comment.
I am not sure if using an std::string for the payload is a good idea.
Member
|
Sorry for taking so long to have a look at this. Apart from fixing the clang-format and dropping the various cout, I think the main issue is that you are using a std::string for the payload. Why is that? Could you move to a preallocated buffer with the correct size? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This change adds a downloader capable of parallel and asynchronous curl calls, with the ability to make callback functions after a request is finished.
A lot of the code is base on an example from the curl manual which combines libuv with libcurl (https://curl.se/libcurl/c/multi-uv.html)
The main differences between the example and this code are:
In case of any questions or remarks please contact me at trzebuniak@student.agh.edu.pl