hard crash if one Xpress function not found #116
Conversation
|
The latest development by google make the Python 3.8 CI actions to crash. |
| @@ -244,7 +236,9 @@ absl::Status LoadXpressDynamicLibrary(std::string& xpresspath) { | |||
| } | |||
|
|
|||
| if (xpress_library.LibraryIsLoaded()) { | |||
| xpress_load_status = LoadXpressFunctions(&xpress_library); | |||
| LOG(INFO) << "Loading all Xpress functions"; | |||
| LoadXpressFunctions(&xpress_library); | |||
There was a problem hiding this comment.
Error handling isn't so great here. Shouldn't we return an absl::status here ?
There was a problem hiding this comment.
I cannot find a way to do it.
LoadXpressFunctions calls DynamicLibrary.GetFunction(const char* function_name)
DynamicLibrary.GetFunction(const char* function_name) executes the CHECK macro that seems to throw a fatal error. I tried to catch it but i probably did not figure out how to do it properly.
Gurobi interface uses the same approach you see here and in the end we can consider that
not having Xpress installed is ok
having the wrong xpress installation is worse
There was a problem hiding this comment.
I am open to change this behavior but at the moment I cannot see how to do it without touching the dynamic_library.h file
There was a problem hiding this comment.
If Google are open to change, it would be easier to suggest an overhaul of dynamic_library.h like we had before
|
Suggestion: |
I have tried it, but I can't figure out how to catch the exception thrown by the I reproduced the same pattern used for the Gurobi interface |
ortools by google uses a dynamic_library that performs a hard check on the successful load of every function.
RTE developed a different, softer approach that is more refined.
Here I propose to revert to the google version and drop RTE version in order for our development to be more easily accepted in the official repository.