[PATCH][RFC] remove retry from http.c

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH][RFC] remove retry from http.c

amalysh
Hi all,

attached patch removes retry within http.c in client code. I think that
it's bad idea to do any retries in http.c module because it's just
library that should just give error to the caller back and not decide to
do any retries. The caller is then responsible to do retry.

Any objections?

Thanks,
Alex

Index: gwlib/http.c
===================================================================
RCS file: /home/cvs/gateway/gwlib/http.c,v
retrieving revision 1.237
diff -a -u -p -r1.237 http.c
--- gwlib/http.c 10 Feb 2006 17:44:53 -0000 1.237
+++ gwlib/http.c 10 Feb 2006 18:06:49 -0000
@@ -682,7 +682,6 @@ typedef struct {
     Connection *conn;
     Octstr *host;
     long port;
-    int retrying;
     int follow_remaining;
     Octstr *certkeyfile;
     int ssl;
@@ -717,7 +716,6 @@ static HTTPServer *server_create(HTTPCal
     trans->port = 0;
     trans->username = NULL;
     trans->password = NULL;
-    trans->retrying = 0;
     trans->follow_remaining = follow_remaining;
     trans->certkeyfile = octstr_duplicate(certkeyfile);
     trans->ssl = 0;
@@ -1005,84 +1003,70 @@ static void handle_transaction(Connectio
     }
 
     while (trans->state != transaction_done) {
-        
         switch (trans->state) {
-        
-            case connecting:
-                debug("gwlib.http", 0, "Get info about connecting socket");
-                if (conn_get_connect_result(trans->conn) != 0) {
-                    debug("gwlib.http", 0, "Socket not connected");
-                    goto error;
-                }
+        case connecting:
+            debug("gwlib.http", 0, "Get info about connecting socket");
+            if (conn_get_connect_result(trans->conn) != 0) {
+                debug("gwlib.http", 0, "Socket not connected");
+                goto error;
+            }
 
-                if ((rc = send_request(trans)) == 0) {
-                    trans->state = reading_status;
-                } else {
-                    debug("gwlib.http",0,"Failed while sending request");
-                    goto error;
-                }
+            if ((rc = send_request(trans)) == 0) {
+                trans->state = reading_status;
+            } else {
+                debug("gwlib.http", 0, "Failed while sending request");
+                goto error;
+            }
             break;
 
-            case reading_status:
-                ret = client_read_status(trans);
-                if (ret < 0) {
-                    /*
-                     * Couldn't read the status from the socket. This may mean
-                     * that the socket had been closed by the server after an
-                     * idle timeout, so we close the connection and try again,
-                     * opening a new socket, but only once.
-                     */
-                    if (trans->retrying) {
-                        debug("gwlib.http", 0, "Failed while retrying");
-                        goto error;
-                    } else {
-                        conn_unregister(trans->conn);
-                        conn_destroy(trans->conn);
-                        trans->conn = NULL;
-                        trans->retrying = 1;
-                        trans->state = request_not_sent;
-                        gwlist_produce(pending_requests, trans);
-                        return;
-                    }
-                } else if (ret == 0) {
-                    /* Got the status, go read headers and body next. */
-                    trans->state = reading_entity;
-                    trans->response = entity_create(
-                        response_expectation(trans->method, trans->status));
-                } else
-                    return;
-                break;
+ case reading_status:
+    ret = client_read_status(trans);
+    if (ret < 0) {
+ /*
+ * Couldn't read the status from the socket. This may mean
+ * that the socket had been closed by the server after an
+ * idle timeout.
+ */
+                debug("gwlib.http",0,"Failed while reading status");
+                goto error;
+    } else if (ret == 0) {
+ /* Got the status, go read headers and body next. */
+ trans->state = reading_entity;
+ trans->response =
+    entity_create(response_expectation(trans->method, trans->status));
+    } else
+ return;
+    break;
     
-            case reading_entity:
-                ret = entity_read(trans->response, conn);
-                if (ret < 0) {
-                    debug("gwlib.http", 0, "Failed reading entity");
-                    goto error;
-                } else if (ret == 0 &&
+ case reading_entity:
+    ret = entity_read(trans->response, conn);
+    if (ret < 0) {
+        debug("gwlib.http",0,"Failed reading entity");
+        goto error;
+    } else if (ret == 0 &&
                     http_status_class(trans->status) == HTTP_STATUS_PROVISIONAL) {
-                    /* This was a provisional reply; get the real one now. */
-                    trans->state = reading_status;
-                    entity_destroy(trans->response);
-                    trans->response = NULL;
-                } else if (ret == 0) {
-                    trans->state = transaction_done;
+                /* This was a provisional reply; get the real one now. */
+                trans->state = reading_status;
+                entity_destroy(trans->response);
+                trans->response = NULL;
+            } else if (ret == 0) {
+                trans->state = transaction_done;
 #ifdef DUMP_RESPONSE
-                    /* Dump the response */
-                    debug("gwlib.http", 0, "HTTP: Received response:");
-                    h = build_response(trans->response->headers, trans->response->body);
-                    octstr_dump(h, 0);
-                    octstr_destroy(h);
+                /* Dump the response */
+                debug("gwlib.http", 0, "HTTP: Received response:");
+                h = build_response(trans->response->headers, trans->response->body);
+                octstr_dump(h, 0);
+                octstr_destroy(h);
 #endif
-                } else {
-                    return;
-                }
-                break;
+    } else {
+                return;
+            }
+            break;
 
-            default:
-                panic(0, "Internal error: Invalid HTTPServer state.");
-        
-        } /* switch */
-    } /* while */
+        default:
+            panic(0, "Internal error: Invalid HTTPServer state.");
+        }
+    }
 
     conn_unregister(trans->conn);
 
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH][RFC] remove retry from http.c

amalysh
Hi,

as no objections were here, commited to cvs.

Thanks,
Alex

Alexander Malysh schrieb:

> Hi all,
>
> attached patch removes retry within http.c in client code. I think that
> it's bad idea to do any retries in http.c module because it's just
> library that should just give error to the caller back and not decide to
> do any retries. The caller is then responsible to do retry.
>
> Any objections?
>
> Thanks,
> Alex
>


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH][RFC] remove retry from http.c

Stipe Tolj
Alexander Malysh wrote:

> Hi,
>
> as no objections were here, commited to cvs.

any implications regarding user's doc? Is this actually a behaviour change I
should mention in the NEWS file for 1.4.1?

Stipe

-------------------------------------------------------------------
Kölner Landstrasse 419
40589 Düsseldorf, NRW, Germany

tolj.org system architecture      Kannel Software Foundation (KSF)
http://www.tolj.org/              http://www.kannel.org/

mailto:st_{at}_tolj.org           mailto:stolj_{at}_kannel.org
-------------------------------------------------------------------

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH][RFC] remove retry from http.c

amalysh
Hi,

Stipe Tolj schrieb:
> Alexander Malysh wrote:
>
>> Hi,
>>
>> as no objections were here, commited to cvs.
>
> any implications regarding user's doc? Is this actually a behaviour
> change I should mention in the NEWS file for 1.4.1?

this change should be invisible to users.

>
> Stipe
>
> -------------------------------------------------------------------
> Kölner Landstrasse 419
> 40589 Düsseldorf, NRW, Germany
>
> tolj.org system architecture      Kannel Software Foundation (KSF)
> http://www.tolj.org/              http://www.kannel.org/
>
> mailto:st_{at}_tolj.org           mailto:stolj_{at}_kannel.org
> -------------------------------------------------------------------
>
>


Reply | Threaded
Open this post in threaded view
|

CVS doesnt compile

Rene Kluwen
Alex,

You changed the function conn_write_data, removing the unsiged modifier from
the char *data parameter in gwlib/conn.c.
You need to update gwlib/conn.h accordingly; and commit to CVS.

Rene Kluwen
Chimit


Reply | Threaded
Open this post in threaded view
|

Re: CVS doesnt compile

Stipe Tolj
Rene Kluwen wrote:

> You changed the function conn_write_data, removing the unsiged modifier from
> the char *data parameter in gwlib/conn.c.
> You need to update gwlib/conn.h accordingly; and commit to CVS.

yep, I fixed cvs, leaving the unsigned char as in previous revision. Thanks Rene.

Stipe

-------------------------------------------------------------------
Kölner Landstrasse 419
40589 Düsseldorf, NRW, Germany

tolj.org system architecture      Kannel Software Foundation (KSF)
http://www.tolj.org/              http://www.kannel.org/

mailto:st_{at}_tolj.org           mailto:stolj_{at}_kannel.org
-------------------------------------------------------------------