[Bug 756318] souphttpclientsink: Add "retry" for request failures

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=756318

Sebastian Dröge (slomo) <slomo at coaxion.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #312985|none                        |needs-work
             status|                            |

--- Comment #1 from Sebastian Dröge (slomo) <slomo at coaxion.net> ---
Review of attachment 312985:
 --> (https://bugzilla.gnome.org/review?bug=756318&attachment=312985)

::: ext/soup/gstsouphttpclientsink.c
@@ +176,3 @@
           G_TYPE_STRV, G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS));
+  g_object_class_install_property (gobject_class, PROP_RETRY,
+      g_param_spec_int ("retry", "Retry",

retry-delay might be better

@@ +753,2 @@
   if (!SOUP_STATUS_IS_SUCCESSFUL (msg->status_code)) {
+    if (souphttpsink->retry > 0) {

Why only look at the header if the property was set? Shouldn't we always look
at the header and respect it? (Also is this also relevant for souphttpsrc?)

@@ +758,3 @@
+          "Retry-After");
+      if (retry_after) {
+        retry = atoi (retry_after);

g_ascii_strtoull() might be better here

@@ +760,3 @@
+        retry = atoi (retry_after);
+        if (retry <= 0) {
+          retry = souphttpsink->retry;

Should this just be a MAX(retry, souphttpsink->retry)?

@@ +771,3 @@
+            msg->reason_phrase, retry);
+      }
+      g_usleep (G_USEC_PER_SEC * retry);

Don't use g_usleep() like that, it's not interruptible. If nothing else is
available for asynchronous waiting here, use a GCond and make sure to unlock it
when needed

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.