{"thread":{"id":"29924","subject":"[PATCH v4 5/5] http: rename HTTP_REAUTH to HTTP_RETRY","startedAt":"2012-03-12T17:30:00Z","lastAt":"2012-03-14T18:19:55Z","messageCount":8,"participants":["Nelson Benitez Leon","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":4,"patchTotal":5},"messages":[{"id":"186738","messageId":"4F5E3298.5030502@seap.minhap.es","threadId":"29924","inReplyTo":null,"subject":"[PATCH v4 5/5] http: rename HTTP_REAUTH to HTTP_RETRY","fromName":"Nelson Benitez Leon","fromEmail":"nelsonjesus.benitez@seap.minhap.es","sentAt":"2012-03-12T17:30:00Z","receivedAt":"2012-03-12T17:30:00Z","isPatch":true,"sender":{"key":"nelsonjesus.benitez@seap.minhap.es","avatar":null},"body":"After adding the proxy authentication support in\nhttp, the semantics of HTTP_REAUTH changed more to\na retry rather than a re-authentication, so we\nrename it to HTTP_RETRY.\n\nSigned-off-by: Nelson Benitez Leon \n---\n http.c | 6 +++---\n http.h | 2 +-\n 2 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex 9b98179..4aa5a46 100644\n--- a/http.c\n+++ b/http.c\n@@ -823,7 +823,7 @@ static int http_request(const char *url, void *result, int target, int options)\n \t\t\t} else {\n \t\t\t\tcredential_fill(&http_auth);\n \t\t\t\tinit_curl_http_auth(slot->curl);\n-\t\t\t\tret = HTTP_REAUTH;\n+\t\t\t\tret = HTTP_RETRY;\n \t\t\t}\n \t\t} else if (results.http_code == 407) { /* Proxy authentication failure */\n \t\t\tif (proxy_auth.username && proxy_auth.password) {\n@@ -832,7 +832,7 @@ static int http_request(const char *url, void *result, int target, int options)\n \t\t\t} else {\n \t\t\t\tcredential_fill(&proxy_auth);\n \t\t\t\tset_proxy_auth(slot->curl);\n-\t\t\t\tret = HTTP_REAUTH;\n+\t\t\t\tret = HTTP_RETRY;\n \t\t\t}\n \t\t} else {\n \t\t\tif (!curl_errorstr[0])\n@@ -862,7 +862,7 @@ static int http_request_reauth(const char *url, void *result, int target,\n \n \tdo {\n \t\tret = http_request(url, result, target, options);\n-\t} while (ret == HTTP_REAUTH);\n+\t} while (ret == HTTP_RETRY);\n \n \treturn ret;\n }\ndiff --git a/http.h b/http.h\nindex 0b61653..6499397 100644\n--- a/http.h\n+++ b/http.h\n@@ -123,7 +123,7 @@ extern char *get_remote_object_url(const char *url, const char *hex,\n #define HTTP_MISSING_TARGET\t1\n #define HTTP_ERROR\t\t2\n #define HTTP_START_FAILED\t3\n-#define HTTP_REAUTH\t4\n+#define HTTP_RETRY\t4\n #define HTTP_NOAUTH\t5\n \n /*\n-- \n1.7.7.6\n"},{"id":"186772","messageId":"7vk42pr3c7.fsf@alter.siamese.dyndns.org","threadId":"29924","inReplyTo":"4F5E3298.5030502@seap.minhap.es","subject":"Re: [PATCH v4 5/5] http: rename HTTP_REAUTH to HTTP_RETRY","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-12T20:06:48Z","receivedAt":"2012-03-12T20:06:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Whatever new token you use, please keep AUTH as a substring of it.\n\nWe may want to retry a request to deal with intermittent failures on\nthe server side or the network between us and the server; HTTP_RETRY\nwould be a good name to signal such condition after we see a failure\nresponse from the library.\n"},{"id":"186831","messageId":"4F5F41FF.4000204@seap.minhap.es","threadId":"29924","inReplyTo":"7vk42pr3c7.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v4 5/5] http: rename HTTP_REAUTH to HTTP_RETRY","fromName":"Nelson Benitez Leon","fromEmail":"nelsonjesus.benitez@seap.minhap.es","sentAt":"2012-03-13T12:47:59Z","receivedAt":"2012-03-13T12:47:59Z","isPatch":true,"sender":{"key":"nelsonjesus.benitez@seap.minhap.es","avatar":null},"body":"On 03/12/2012 09:06 PM, Junio C Hamano wrote:\n> Whatever new token you use, please keep AUTH as a substring of it.\n> \n> We may want to retry a request to deal with intermittent failures on\n> the server side or the network between us and the server; HTTP_RETRY\n> would be a good name to signal such condition after we see a failure\n> response from the library.\n\nHTTP_REAUTH and HTTP_AUTH_RETRY seems like the same thing, so imo not \ndeserving the rename, maybe Jeff can suggest a better name as he was\nwho suggest the rename.\n"},{"id":"186881","messageId":"7vy5r4wfru.fsf@alter.siamese.dyndns.org","threadId":"29924","inReplyTo":"4F5F41FF.4000204@seap.minhap.es","subject":"Re: [PATCH v4 5/5] http: rename HTTP_REAUTH to HTTP_RETRY","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-13T17:51:33Z","receivedAt":"2012-03-13T17:51:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nelson Benitez Leon writes:\n\n> On 03/12/2012 09:06 PM, Junio C Hamano wrote:\n>> Whatever new token you use, please keep AUTH as a substring of it.\n>> \n>> We may want to retry a request to deal with intermittent failures on\n>> the server side or the network between us and the server; HTTP_RETRY\n>> would be a good name to signal such condition after we see a failure\n>> response from the library.\n>\n> HTTP_REAUTH and HTTP_AUTH_RETRY seems like the same thing, so imo not \n> deserving the rename, maybe Jeff can suggest a better name as he was\n> who suggest the rename.\n\nEither has AUTH as a substring in it, and leaves a door open for us to\nlater introduce HTTP_RETRY to tell the machinery that drives cURL library\nto retry the request, so in that sense I am OK with either, but as your\nlog message said, we want to make it clear that this is not about doing\nthe authentication again (re-auth) but retrying the authentication, so\nHTTP_AUTH_RETRY would be more logical name.\n"},{"id":"186909","messageId":"20120313220411.GA28357@sigill.intra.peff.net","threadId":"29924","inReplyTo":"7vy5r4wfru.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v4 5/5] http: rename HTTP_REAUTH to HTTP_RETRY","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-03-13T22:04:11Z","receivedAt":"2012-03-13T22:04:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 13, 2012 at 10:51:33AM -0700, Junio C Hamano wrote:\n\n> > On 03/12/2012 09:06 PM, Junio C Hamano wrote:\n> >> Whatever new token you use, please keep AUTH as a substring of it.\n> >> \n> >> We may want to retry a request to deal with intermittent failures on\n> >> the server side or the network between us and the server; HTTP_RETRY\n> >> would be a good name to signal such condition after we see a failure\n> >> response from the library.\n> >\n> > HTTP_REAUTH and HTTP_AUTH_RETRY seems like the same thing, so imo not \n> > deserving the rename, maybe Jeff can suggest a better name as he was\n> > who suggest the rename.\n> \n> Either has AUTH as a substring in it, and leaves a door open for us to\n> later introduce HTTP_RETRY to tell the machinery that drives cURL library\n> to retry the request, so in that sense I am OK with either, but as your\n> log message said, we want to make it clear that this is not about doing\n> the authentication again (re-auth) but retrying the authentication, so\n> HTTP_AUTH_RETRY would be more logical name.\n\nI suggested RETRY because that is all the caller needs to know: the\nhttp_request machinery said \"please call me again\". Keep in mind that\nthis is a private interface within http.c, and this return code should\nnever make it out at all. Nor is it something anybody else would feed\nus.\n\nI am half-tempted to suggest refactoring it to return the actual error\ncode, and let the caller handle 401 and 407. That would be more readable\noverall, I think. But it's a little complicated, because getting the\nexact answer depends on the curl handle, which is local to http_request,\nand I don't want to hold Nelson's actual feature improvement hostage to\nsuch refactoring.\n\n-Peff\n"},{"id":"186913","messageId":"7v1uowt83u.fsf@alter.siamese.dyndns.org","threadId":"29924","inReplyTo":"20120313220411.GA28357@sigill.intra.peff.net","subject":"Re: [PATCH v4 5/5] http: rename HTTP_REAUTH to HTTP_RETRY","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-13T23:05:25Z","receivedAt":"2012-03-13T23:05:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Tue, Mar 13, 2012 at 10:51:33AM -0700, Junio C Hamano wrote:\n>\n>> Either has AUTH as a substring in it, and leaves a door open for us to\n>> later introduce HTTP_RETRY to tell the machinery that drives cURL library\n>> to retry the request, so in that sense I am OK with either, but as your\n>> log message said, we want to make it clear that this is not about doing\n>> the authentication again (re-auth) but retrying the authentication, so\n>> HTTP_AUTH_RETRY would be more logical name.\n>\n> I suggested RETRY because that is all the caller needs to know: the\n> http_request machinery said \"please call me again\". Keep in mind that\n> this is a private interface within http.c, and this return code should\n> never make it out at all. Nor is it something anybody else would feed\n> us.\n\nOh, the potential \"retry when a request failed\" in the future I had in\nmind was also contained within http.c. Perhaps HTTP_RETRY could be used\nfor the same purpose? The places I had in mind that we may potentially\nwant to retry are where we got 50x from one of the servers in the pool\nthat serves the name we are accessing, we got 401 from the server to let\nus realize we gave it a wrong credential, or we got 407 from the proxy to\nnotify a similar situation, and all are potential candidate for retrying\nin the client may help. The credential might have been mistyped for 40x,\nor we may hit a healthy server in the same pool for 50x.\n"},{"id":"186927","messageId":"4F607CEF.5010209@seap.minhap.es","threadId":"29924","inReplyTo":"7v1uowt83u.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v4 5/5] http: rename HTTP_REAUTH to HTTP_RETRY","fromName":"Nelson Benitez Leon","fromEmail":"nelsonjesus.benitez@seap.minhap.es","sentAt":"2012-03-14T11:11:43Z","receivedAt":"2012-03-14T11:11:43Z","isPatch":true,"sender":{"key":"nelsonjesus.benitez@seap.minhap.es","avatar":null},"body":"After adding the proxy authentication support in\nhttp, the semantics of HTTP_REAUTH changed more to\na retry rather than a re-authentication, so we\nrename it to HTTP_AUTH_RETRY.\n\nSigned-off-by: Nelson Benitez Leon \n---\nOk this is a new 5/5 patch that have HTTP_AUTH_RETRY as\nJunio suggested, is responding with this patch good or\ndo I need to send a new re-roll just for this?\n\nthanks, \n\n http.c | 6 +++---\n http.h | 2 +-\n 2 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex 12dcaa1..7468cdb 100644\n--- a/http.c\n+++ b/http.c\n@@ -837,7 +837,7 @@ static int http_request(const char *url, void *result, int target, int options)\n \t\t\t} else {\n \t\t\t\tcredential_fill(&http_auth);\n \t\t\t\tinit_curl_http_auth(slot->curl);\n-\t\t\t\tret = HTTP_REAUTH;\n+\t\t\t\tret = HTTP_AUTH_RETRY;\n \t\t\t}\n \t\t} else if (results.http_code == 407) { /* Proxy authentication failure */\n \t\t\tif (proxy_auth.username && proxy_auth.password) {\n@@ -846,7 +846,7 @@ static int http_request(const char *url, void *result, int target, int options)\n \t\t\t} else {\n \t\t\t\tcredential_fill(&proxy_auth);\n \t\t\t\tset_proxy_auth(slot->curl);\n-\t\t\t\tret = HTTP_REAUTH;\n+\t\t\t\tret = HTTP_AUTH_RETRY;\n \t\t\t}\n \t\t} else {\n \t\t\tif (!curl_errorstr[0])\n@@ -876,7 +876,7 @@ static int http_request_reauth(const char *url, void *result, int target,\n \n \tdo {\n \t\tret = http_request(url, result, target, options);\n-\t} while (ret == HTTP_REAUTH);\n+\t} while (ret == HTTP_AUTH_RETRY);\n \n \treturn ret;\n }\ndiff --git a/http.h b/http.h\nindex 303eafb..6e3ea59 100644\n--- a/http.h\n+++ b/http.h\n@@ -123,7 +123,7 @@ extern char *get_remote_object_url(const char *url, const char *hex,\n #define HTTP_MISSING_TARGET\t1\n #define HTTP_ERROR\t\t2\n #define HTTP_START_FAILED\t3\n-#define HTTP_REAUTH\t4\n+#define HTTP_AUTH_RETRY\t4\n #define HTTP_NOAUTH\t5\n \n /*\n-- \n1.7.7.6\n"},{"id":"186972","messageId":"7vfwdbqc38.fsf@alter.siamese.dyndns.org","threadId":"29924","inReplyTo":"4F607CEF.5010209@seap.minhap.es","subject":"Re: [PATCH v4 5/5] http: rename HTTP_REAUTH to HTTP_RETRY","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-14T18:19:55Z","receivedAt":"2012-03-14T18:19:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nelson Benitez Leon writes:\n\n> After adding the proxy authentication support in\n> http, the semantics of HTTP_REAUTH changed more to\n> a retry rather than a re-authentication, so we\n> rename it to HTTP_AUTH_RETRY.\n>\n> Signed-off-by: Nelson Benitez Leon \n> ---\n> Ok this is a new 5/5 patch that have HTTP_AUTH_RETRY as\n> Junio suggested, is responding with this patch good or\n> do I need to send a new re-roll just for this?\n\nHeh, HTTP_AUTH_RETRY was not something I suggested ;-) The name comes from\nyour http://mid.gmane.org/4F5F41FF.4000204@seap.minhap.es message.\n\nRegarding whether to re-send everything or only a selected subset, please\nfollow your best judgement, like you did this time.\n\nIn general, when you know that the participants in the discussion are\nkeeping closer eyes on the progress of the series, and they are likely to\nunderstand what you mean when you say \"I am replacing the last one in the\nv3 series I sent earlier with this patch\" when you send \"[PATCH v4 5/5]\",\nit is appropriate to send only the updated one(s).\n\nIt makes only two small differences if I am or I am not among the\nparticipants in the discussion.\n\n - When I happen to be involved in a topic and keeping closer eyes on it,\n an earlier iteration of it is likely to appear on 'pu', so you have one\n more clue to tell if it is OK to send just an update, compared to a\n series that is discussed only on the list without anybody tracking the\n most recent state of the series.\n\n - When I am not involved in a discussion, often I am sitting on the\n sideline (a recent example is the topic around svn-fe/fast-import\n regarding \"ls\" command on an empty path), letting the stakeholders in\n the series figure out the details and waiting for the final outcome of\n the discussion [*1*]. For such a topic, I may request a full resend of\n the final version when it is time for me to queue it.\n\nThanks. I replaced the corresponding patch with this, after fixing the\nsubject line.\n\n[Footnote]\n\n*1* This happens when I have more confidence in them than in myself to\njudge the best direction for the series.\n"}]}