Encapsulating the use of CURL libraries #176
Conversation
There was a problem hiding this comment.
Looks good to me, but I think all this logic should be on the framework library directory, not on the root directory, even though it does the same thing this is only because we add this subdirectory first. I would do something like what I suggested here, if you agree.
|
Plz @lobis commit to apply the changes as you propose directly to this PR. |
…`add_definitions` and fixed bad usage of `find_library` when not found
Now I think it should work, previously it was trying to add the library even if it didn't find it. It would be nice to test it on a system without this library before merging though. |
|
I tested by changing the name to a library I dont have installed, and it was working |
|
Perhaps at NAF-IAXO is not yet installed, although I requested it. It is |
|
For me at NAF-IAXO (where CURL libraries are not installed) at commit dfd08b2 works properly, perhaps the problem was introduced later on. |
Perhaps you got issues with previously cached cmake variables at your build environment? |
oh okay, I was just using the documentation as reference, the variable when not defined is not |
| std::getline(std::ifstream(filename), file_content, '\0'); | ||
| #else | ||
| ferr << "TRestTools::POSTRequest. REST framework was compiled without CURL support" << endl; |
There was a problem hiding this comment.
I think we should create a RESTException for these cases. Shall I create an issue about this?
There was a problem hiding this comment.
Yes, if you like, please, explain at the issue how do you want to handle it. For the moment we use no expections, as you see, as it is it just prompts a warning message, then does nothing. Not sure if in this particular case it requires of error handling.
There was a problem hiding this comment.
I would say wherever we have exit() in the code we should replace by a throw with an exception handler. Perhaps not for this particular case...
Yes, in my case it was also the format |
oh, then we should use that instead! |
We recently included dependency with curl libraries.
Inside this PR I am encapsulating the methods using CURL so that if no curl libraries have been installed in the system the CURL functions used in the code are bypassed.
Perhaps cmake gurus prefer a different add-on. In that case, please do not hesitate to commit any changes here.
@rest-for-physics/core_dev