)]}'
{"swift/cli/recon.py":[{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"adc5cd4e564ca4b1f419c9bfa1b7d36836f18f39","unresolved":false,"context_lines":[{"line_number":114,"context_line":"        Obtain Server header by calling OPTIONS."},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"        :param host: host to check"},{"line_number":117,"context_line":"        :returns: Server type, status"},{"line_number":118,"context_line":"        \"\"\""},{"line_number":119,"context_line":"        try:"},{"line_number":120,"context_line":"            url \u003d \"http://%s:%s/\" % (host[0], host[1])"}],"source_content_type":"text/x-python","patch_set":4,"id":"da86d52c_b5246760","line":117,"updated":"2015-02-12 21:53:00.000000000","message":"Update the returns to match what is returned.","commit_id":"4f4d1947b03d9cc4c61763c1d17fab8648e900d1"},{"author":{"_account_id":330,"name":"John Dickinson","email":"me@not.mn","username":"notmyname"},"change_message_id":"4a807d03eee551c1263eaa7dac6a605ec34d7e59","unresolved":false,"context_lines":[{"line_number":372,"context_line":"        recon \u003d Scout(\"server_type_check\", self.verbose, self.suppress_errors,"},{"line_number":373,"context_line":"                      self.timeout)"},{"line_number":374,"context_line":"        print(\"[%s] Verifying server types of %s hosts...\" %"},{"line_number":375,"context_line":"              (self._ptime(), len(hosts)))"},{"line_number":376,"context_line":"        if self.server_type \u003d\u003d \u0027object\u0027:"},{"line_number":377,"context_line":"            for url, response in self.pool.imap(recon.scout_server_type,"},{"line_number":378,"context_line":"                                                hosts):"}],"source_content_type":"text/x-python","patch_set":4,"id":"da86d52c_95092397","line":375,"updated":"2015-02-12 21:48:17.000000000","message":"would be nice to have the server type you\u0027re validating here","commit_id":"4f4d1947b03d9cc4c61763c1d17fab8648e900d1"},{"author":{"_account_id":330,"name":"John Dickinson","email":"me@not.mn","username":"notmyname"},"change_message_id":"4a807d03eee551c1263eaa7dac6a605ec34d7e59","unresolved":false,"context_lines":[{"line_number":374,"context_line":"        print(\"[%s] Verifying server types of %s hosts...\" %"},{"line_number":375,"context_line":"              (self._ptime(), len(hosts)))"},{"line_number":376,"context_line":"        if self.server_type \u003d\u003d \u0027object\u0027:"},{"line_number":377,"context_line":"            for url, response in self.pool.imap(recon.scout_server_type,"},{"line_number":378,"context_line":"                                                hosts):"},{"line_number":379,"context_line":"                if response \u003d\u003d \u0027container-server\u0027:"},{"line_number":380,"context_line":"                    errors[url].append(\u0027container-server\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"da86d52c_15ce135a","line":377,"updated":"2015-02-12 21:48:17.000000000","message":"needs \"url, response, status\" (or something). The method is returning 3 values, not 2. Same for the other 2 calls below.","commit_id":"4f4d1947b03d9cc4c61763c1d17fab8648e900d1"},{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"adc5cd4e564ca4b1f419c9bfa1b7d36836f18f39","unresolved":false,"context_lines":[{"line_number":379,"context_line":"                if response \u003d\u003d \u0027container-server\u0027:"},{"line_number":380,"context_line":"                    errors[url].append(\u0027container-server\u0027)"},{"line_number":381,"context_line":"                if response \u003d\u003d \u0027account-server\u0027:"},{"line_number":382,"context_line":"                    errors[url].append(\u0027account-server\u0027)"},{"line_number":383,"context_line":"            for host in errors:"},{"line_number":384,"context_line":"                print(\"Invalid: %s is a %s\" % (host, errors[host]))"},{"line_number":385,"context_line":"        if self.server_type \u003d\u003d \u0027container\u0027:"}],"source_content_type":"text/x-python","patch_set":4,"id":"da86d52c_35bcf705","line":382,"updated":"2015-02-12 21:53:00.000000000","message":"To avoid possible typos and copy/paste mistakes later down the road, you could use errors[url].append(response) since the response string is identical to what you want to append.  Furthermore, if you use that, then you could combine these two cases into one.\n\nEven better, you could collapse these three if blocks that are on server_type to check:\nif response !\u003d self.server_type + \u0027-server\u0027:\n    errors[url].append(response)\n\nAll the other lines in the 3 blocks are the same, so you would then only need one block.","commit_id":"4f4d1947b03d9cc4c61763c1d17fab8648e900d1"},{"author":{"_account_id":12261,"name":"Mahati","email":"mahati.chamarthy@gmail.com","username":"Mahati"},"change_message_id":"646c65796069c1513e1c81d838cdac4a6de9456c","unresolved":false,"context_lines":[{"line_number":379,"context_line":"                if response \u003d\u003d \u0027container-server\u0027:"},{"line_number":380,"context_line":"                    errors[url].append(\u0027container-server\u0027)"},{"line_number":381,"context_line":"                if response \u003d\u003d \u0027account-server\u0027:"},{"line_number":382,"context_line":"                    errors[url].append(\u0027account-server\u0027)"},{"line_number":383,"context_line":"            for host in errors:"},{"line_number":384,"context_line":"                print(\"Invalid: %s is a %s\" % (host, errors[host]))"},{"line_number":385,"context_line":"        if self.server_type \u003d\u003d \u0027container\u0027:"}],"source_content_type":"text/x-python","patch_set":4,"id":"da86d52c_4b5e037b","line":382,"in_reply_to":"da86d52c_35bcf705","updated":"2015-02-13 16:37:24.000000000","message":"Hi, thanks for the review! I agree with replacing the word with \"response\" (to avoid copy/paste errors). But the latter suggestion of \"response !\u003d self.server_type + \u0027-server\u0027:\" would not do well. Because response could even carry an error. I should be doing some error handling. I\u0027m working on that. Thanks!","commit_id":"4f4d1947b03d9cc4c61763c1d17fab8648e900d1"},{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"a2f3a2483754f9ffc041768274ed0038cd48abae","unresolved":false,"context_lines":[{"line_number":372,"context_line":"        recon \u003d Scout(\"server_type_check\", self.verbose, self.suppress_errors,"},{"line_number":373,"context_line":"                      self.timeout)"},{"line_number":374,"context_line":"        print(\"[%s] Validating server type \u0027%s\u0027 on %s hosts...\" %"},{"line_number":375,"context_line":"              (self._ptime(), self.server_type, len(hosts)))"},{"line_number":376,"context_line":"        if self.server_type \u003d\u003d \u0027object\u0027:"},{"line_number":377,"context_line":"            for url, response, status in self.pool.imap("},{"line_number":378,"context_line":"                    recon.scout_server_type, hosts):"}],"source_content_type":"text/x-python","patch_set":5,"id":"da86d52c_366e4aca","line":375,"updated":"2015-02-17 03:21:26.000000000","message":"Continuing the previous comments in this area, I see that the response might be an error string if an exception was thrown in scout_server_type.\nIn that case, the status is not 200.  The current code is only adding to errors if the status \u003d\u003d 200.\n(Looks like that is because the error is already printed if one of the two checked exceptions is thrown.)\n\nI am wondering if the only correct response for \nself.server_type \u003d\u003d \u0027object\u0027 is response \u003d\u003d \u0027object-server\u0027.  \n\nIf so, then you could change the logic to add to errors if the response doesn\u0027t match the server type.\n\nSo effectively, you would end up with the following instead of the  3  conditional blocks:\n\n        print(\"[%s] Validating server type \u0027%s\u0027 on %s hosts...\" %\n              (self._ptime(), self.server_type, len(hosts)))\n\n        for url, response, status in self.pool.imap(\n                    recon.scout_server_type, hosts):\n            if status \u003d\u003d 200:\n                if response !\u003d self.server_type + \u0027-server\u0027:\n                    errors[url] \u003d response\n        if not bool(errors):\n            print(\"No hosts returned invalid data!\")\n        else:\n            for host in errors:\n                print(\"Invalid: %s is %s\" % (host, errors[host]))\n\n        print(\"\u003d\" * 79)","commit_id":"e533bf71a7f55df4adfc10312359bc9e19625117"},{"author":{"_account_id":12261,"name":"Mahati","email":"mahati.chamarthy@gmail.com","username":"Mahati"},"change_message_id":"58645827749aa832deb87b887d78c85a97f7eb81","unresolved":false,"context_lines":[{"line_number":372,"context_line":"        recon \u003d Scout(\"server_type_check\", self.verbose, self.suppress_errors,"},{"line_number":373,"context_line":"                      self.timeout)"},{"line_number":374,"context_line":"        print(\"[%s] Validating server type \u0027%s\u0027 on %s hosts...\" %"},{"line_number":375,"context_line":"              (self._ptime(), self.server_type, len(hosts)))"},{"line_number":376,"context_line":"        if self.server_type \u003d\u003d \u0027object\u0027:"},{"line_number":377,"context_line":"            for url, response, status in self.pool.imap("},{"line_number":378,"context_line":"                    recon.scout_server_type, hosts):"}],"source_content_type":"text/x-python","patch_set":5,"id":"da86d52c_be6e4c94","line":375,"in_reply_to":"da86d52c_366e4aca","updated":"2015-02-18 15:09:10.000000000","message":"Okay, so I think I get it. I see what you mean. (I read it again to double check :) sorry for the confusion!)\n\nYes, it can be done I believe. (and this is my last comment :) )","commit_id":"e533bf71a7f55df4adfc10312359bc9e19625117"},{"author":{"_account_id":12261,"name":"Mahati","email":"mahati.chamarthy@gmail.com","username":"Mahati"},"change_message_id":"96010f6a59b893060a188b33083a602f56f2a356","unresolved":false,"context_lines":[{"line_number":372,"context_line":"        recon \u003d Scout(\"server_type_check\", self.verbose, self.suppress_errors,"},{"line_number":373,"context_line":"                      self.timeout)"},{"line_number":374,"context_line":"        print(\"[%s] Validating server type \u0027%s\u0027 on %s hosts...\" %"},{"line_number":375,"context_line":"              (self._ptime(), self.server_type, len(hosts)))"},{"line_number":376,"context_line":"        if self.server_type \u003d\u003d \u0027object\u0027:"},{"line_number":377,"context_line":"            for url, response, status in self.pool.imap("},{"line_number":378,"context_line":"                    recon.scout_server_type, hosts):"}],"source_content_type":"text/x-python","patch_set":5,"id":"da86d52c_2a1937d1","line":375,"in_reply_to":"da86d52c_366e4aca","updated":"2015-02-18 14:56:52.000000000","message":"You are right about the errors being handled, they are checked in scout_server_type.\nRegarding replacing the two conditional blocks, yes, I can split the string and do the check instead of 2 conditions. But I feel it takes away the readability.\n\nI\u0027m not sure if the performance will be effected too much. Let me see if someone else also feels the same. Thanks for the review!","commit_id":"e533bf71a7f55df4adfc10312359bc9e19625117"},{"author":{"_account_id":14966,"name":"Denis Cavalcante","email":"dencaval@gmail.com","username":"denis.cavalcante"},"change_message_id":"e3a7c7e16574d2b586dcbd39130622ec37e67611","unresolved":false,"context_lines":[{"line_number":373,"context_line":"                      self.timeout)"},{"line_number":374,"context_line":"        print(\"[%s] Validating server type \u0027%s\u0027 on %s hosts...\" %"},{"line_number":375,"context_line":"              (self._ptime(), self.server_type, len(hosts)))"},{"line_number":376,"context_line":"        if self.server_type \u003d\u003d \u0027object\u0027:"},{"line_number":377,"context_line":"            for url, response, status in self.pool.imap("},{"line_number":378,"context_line":"                    recon.scout_server_type, hosts):"},{"line_number":379,"context_line":"                if status \u003d\u003d 200:"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba7be1f8_51de7440","line":376,"updated":"2015-02-23 14:31:09.000000000","message":"You can correct me if I\u0027m wrong, but it seems that \"swift-recon --md5\" command validates for object, container and account rings per execution, but --validate-servers validates only one server-type per execution. \n\nMaybe a loop could be added to reduce duplicated code and all the server types could be validated per execution of \"swift-recon --validate-servers\".\n\nOr \"validate-servers\" could be \"validate-server\".","commit_id":"e533bf71a7f55df4adfc10312359bc9e19625117"},{"author":{"_account_id":14966,"name":"Denis Cavalcante","email":"dencaval@gmail.com","username":"denis.cavalcante"},"change_message_id":"3596ba721724e2ec34cd34d01baa2569fdc6d652","unresolved":false,"context_lines":[{"line_number":917,"context_line":""},{"line_number":918,"context_line":"        print(\"--\u003e Starting reconnaissance on %s hosts\" % len(hosts))"},{"line_number":919,"context_line":"        print(\"\u003d\" * 79)"},{"line_number":920,"context_line":""},{"line_number":921,"context_line":"        if options.all:"},{"line_number":922,"context_line":"            if self.server_type \u003d\u003d \u0027object\u0027:"},{"line_number":923,"context_line":"                self.async_check(hosts)"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba7be1f8_ae8906cc","side":"PARENT","line":920,"updated":"2015-02-23 18:47:40.000000000","message":"I think it\u0027s missing to include \"validate_servers\" to the \"all\" option.","commit_id":"9196c31f6409c5c4f929ddbc71852997f99bee6f"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"4906a0f947fef7304f6c848dfdf79d8b5c72043c","unresolved":false,"context_lines":[{"line_number":380,"context_line":"                if response !\u003d self.server_type + \u0027-server\u0027:"},{"line_number":381,"context_line":"                    errors[url] \u003d response"},{"line_number":382,"context_line":"        if not bool(errors):"},{"line_number":383,"context_line":"            print(\"No hosts returned invalid data!\")"},{"line_number":384,"context_line":"        else:"},{"line_number":385,"context_line":"            for host in errors:"},{"line_number":386,"context_line":"                print(\"Invalid: %s is %s\" % (host, errors[host]))"}],"source_content_type":"text/x-python","patch_set":7,"id":"ba7be1f8_096abbb2","line":383,"updated":"2015-02-25 10:23:21.000000000","message":"First I was confused by this, because it looks very similar to \"No hosts returned valid data.\" that is returned by many other checks.\n\nIt would be nice to return a more explicit message, for example similar to the one returned by the md5 check. How about this one (replacing line 382-386):\n\n print(\"%s/%s hosts ok, %s error[s] while checking hosts.\" % (\n     len(hosts)-len(errors), len(hosts), len(errors)))\n for host in errors:\n     print(\"Invalid: %s is %s\" % (host, errors[host]))","commit_id":"514e52f8a6225785a68fc829f1c9d1bc78a839ce"}],"test/unit/cli/test_recon.py":[{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"4906a0f947fef7304f6c848dfdf79d8b5c72043c","unresolved":false,"context_lines":[{"line_number":321,"context_line":""},{"line_number":322,"context_line":"        return mock.patch(\u0027eventlet.green.urllib2.urlopen\u0027, fake_urlopen)"},{"line_number":323,"context_line":""},{"line_number":324,"context_line":"    def test_server_type_check(self):"},{"line_number":325,"context_line":"        hosts \u003d [(\u0027127.0.0.1\u0027, 6010), (\u0027127.0.0.1\u0027, 6011),"},{"line_number":326,"context_line":"                 (\u0027127.0.0.1\u0027, 6012)]"},{"line_number":327,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"ba7be1f8_69717743","line":324,"updated":"2015-02-25 10:23:21.000000000","message":"I think this test lacks a verification in case everything is ok","commit_id":"514e52f8a6225785a68fc829f1c9d1bc78a839ce"}]}