Make TURN REST API timeout configurable in janus.jcfg #2470
Conversation
turnrest.c
Outdated
| @@ -31,6 +31,7 @@ | |||
| static const char *api_server = NULL; | |||
| static const char *api_key = NULL; | |||
| static gboolean api_http_get = FALSE; | |||
| static uint api_timeout = 10; | |||
There was a problem hiding this comment.
Doesn't look like the default here matters: it's always overwritten in janus_turnrest_set_backend.
| @@ -32,7 +32,7 @@ void janus_turnrest_deinit(void); | |||
| * TURN REST API entirely) | |||
| * @param key The API key, if any (pass NULL if it's not required) | |||
| * @param method The HTTP method to use, POST or GET (NULL means POST) */ | |||
| void janus_turnrest_set_backend(const char *server, const char *key, const char *method); | |||
| void janus_turnrest_set_backend(const char *server, const char *key, const char *method, const uint timeout); | |||
There was a problem hiding this comment.
You need to add the new attribute to the above doxygen documentation too.
janus.c
Outdated
| @@ -4643,6 +4643,7 @@ gint main(int argc, char *argv[]) | |||
| char *turn_rest_api = NULL, *turn_rest_api_key = NULL; | |||
| #ifdef HAVE_TURNRESTAPI | |||
| char *turn_rest_api_method = NULL; | |||
| uint turn_rest_api_timeout = 0; | |||
There was a problem hiding this comment.
It looks like if turn_rest_api_timeout is not set in the config file, the above 0 value is passed to janus_ice_set_turn_rest_api, which then overwrites api_timeout inturnrest.c thus indeed causing the indefinite wait you wanted to prevent.
| @@ -300,6 +300,7 @@ nat: { | |||
| #turn_rest_api = "http://yourbackend.com/path/to/api" | |||
| #turn_rest_api_key = "anyapikeyyoumayhaveset" | |||
| #turn_rest_api_method = "GET" | |||
| #turn_rest_api_timeout = 10 | |||
There was a problem hiding this comment.
The new property is not discussed in the text above. It should be explained that there's a configurable timeout, and that it's in seconds. As it is, people reading the file will just see 10 and not know what it means.
|
Thanks for the quick review, and for catching the error in default handling. Is this a better approach? I also addressed the documentation issues. |
|
Thanks for the quick fixes! I see you signed the CLA and the code, so we can merge 👍 |
Make TURN REST API timeout configurable in janus.jcfg (meetecho#2470)
No description provided.