)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":10063,"name":"Steve McLellan","email":"steven.j.mclellan@gmail.com","username":"sjmc7"},"change_message_id":"b12f7f4520c4bffc9bd26c03df24d55d5f0c67c8","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Fix a defect in the handling for simultaneous,"},{"line_number":10,"context_line":"multiple security group rule updates."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: Ib259777323ec0065ccf54adbab739080936bf5cf"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"fa7ab95a_86ff7800","line":12,"range":{"start_line":12,"start_character":11,"end_line":12,"end_character":52},"updated":"2016-08-26 14:48:19.000000000","message":"I\u0027ve filed https://bugs.launchpad.net/searchlight/+bug/1617364 to track this","commit_id":"05b8fe231580156a305b0124df728a6f11e418c9"},{"author":{"_account_id":14680,"name":"Rick Aulino","email":"rick.aulino@hpe.com","username":"ricka"},"change_message_id":"bf8fa852377520b35cbfb35131e781e9236de4d4","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Fix a defect in the handling for simultaneous,"},{"line_number":10,"context_line":"multiple security group rule updates."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: Ib259777323ec0065ccf54adbab739080936bf5cf"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"fa7ab95a_346fa8c6","line":12,"range":{"start_line":12,"start_character":11,"end_line":12,"end_character":52},"in_reply_to":"fa7ab95a_86ff7800","updated":"2016-08-26 19:28:16.000000000","message":"Added.","commit_id":"05b8fe231580156a305b0124df728a6f11e418c9"},{"author":{"_account_id":10063,"name":"Steve McLellan","email":"steven.j.mclellan@gmail.com","username":"sjmc7"},"change_message_id":"00e4cab3c04f0f0c09d4f6c175a3fd72335cc5eb","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Rick Aulino \u003crick.aulino@hp.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2016-09-06 17:21:08 -0600"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Standard error logging"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Fix a defect in the handling for simultaneous,"},{"line_number":10,"context_line":"multiple security group rule updates."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"9a89bdaa_e5c9175c","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":8},"updated":"2016-09-09 18:43:45.000000000","message":"This is a bit misleading anda  hangover from an earlier patch i think","commit_id":"ef33ca32430c3e1519c45a9b04ba955c4eab398e"},{"author":{"_account_id":10063,"name":"Steve McLellan","email":"steven.j.mclellan@gmail.com","username":"sjmc7"},"change_message_id":"00e4cab3c04f0f0c09d4f6c175a3fd72335cc5eb","unresolved":false,"context_lines":[{"line_number":10,"context_line":"multiple security group rule updates."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: Ib259777323ec0065ccf54adbab739080936bf5cf"},{"line_number":13,"context_line":"Closes-Bug: 1617364"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"9a89bdaa_05c7db45","line":13,"range":{"start_line":13,"start_character":12,"end_line":13,"end_character":19},"updated":"2016-09-09 18:43:45.000000000","message":"Not sure if it\u0027s optional to put the \u0027#\u0027 before the bug number","commit_id":"ef33ca32430c3e1519c45a9b04ba955c4eab398e"}],"searchlight/elasticsearch/plugins/neutron/notification_handlers.py":[{"author":{"_account_id":4428,"name":"Liyingjun","email":"yinjalee@163.com","username":"liyingjun"},"change_message_id":"c495f7c29e3b4915fc2c4a1d51dd04936bcf2533","unresolved":false,"context_lines":[{"line_number":350,"context_line":"        # NB: Most of the looping logic is the same as in \"delete_rule\"."},{"line_number":351,"context_line":"        #     The read/modify the ES document is different. If the logic"},{"line_number":352,"context_line":"        #     changes, please make the changes there."},{"line_number":353,"context_line":"        retries \u003d 20"},{"line_number":354,"context_line":"        group_id \u003d payload[\u0027security_group_rule\u0027][\u0027security_group_id\u0027]"},{"line_number":355,"context_line":"        LOG.debug(\"Updating security group rule information for %s\", group_id)"},{"line_number":356,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"1ac06dbe_4b19cb21","line":353,"range":{"start_line":353,"start_character":8,"end_line":353,"end_character":20},"updated":"2016-08-23 01:57:10.000000000","message":"I suggest making this configurable","commit_id":"ff920ec5571b42bcd14fdf1ed433ce02b454191f"},{"author":{"_account_id":14680,"name":"Rick Aulino","email":"rick.aulino@hpe.com","username":"ricka"},"change_message_id":"c465c0b69d4b992cc32fa32a65e20a3aa53d8004","unresolved":false,"context_lines":[{"line_number":350,"context_line":"        # NB: Most of the looping logic is the same as in \"delete_rule\"."},{"line_number":351,"context_line":"        #     The read/modify the ES document is different. If the logic"},{"line_number":352,"context_line":"        #     changes, please make the changes there."},{"line_number":353,"context_line":"        retries \u003d 20"},{"line_number":354,"context_line":"        group_id \u003d payload[\u0027security_group_rule\u0027][\u0027security_group_id\u0027]"},{"line_number":355,"context_line":"        LOG.debug(\"Updating security group rule information for %s\", group_id)"},{"line_number":356,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"fa7ab95a_9fa7ec4e","line":353,"range":{"start_line":353,"start_character":8,"end_line":353,"end_character":20},"in_reply_to":"1ac06dbe_4b19cb21","updated":"2016-08-25 23:54:11.000000000","message":"I thought about this. If there is a hard-coded value, I also prefer to allow it to be over-ridden by a variable in a config file. In this case, I\u0027m not sure if anyone will ever modify this value.\n\nAs such, it seems that adding it to the config file will just add to the noise of the config file. More documentation that the user has to go through, etc. For this case, I didn\u0027t think it was worth adding, I thought it would be more confusing.\n\nIf you still want it added to the config file, let me know.","commit_id":"ff920ec5571b42bcd14fdf1ed433ce02b454191f"},{"author":{"_account_id":10063,"name":"Steve McLellan","email":"steven.j.mclellan@gmail.com","username":"sjmc7"},"change_message_id":"b12f7f4520c4bffc9bd26c03df24d55d5f0c67c8","unresolved":false,"context_lines":[{"line_number":376,"context_line":"                    # Conflict error, retry with new version of doc."},{"line_number":377,"context_line":"                    pass"},{"line_number":378,"context_line":"                else:"},{"line_number":379,"context_line":"                    LOG.error(_LE(\u0027Error adding security group rule %(id)s\u0027) %"},{"line_number":380,"context_line":"                              {\u0027id\u0027: group_id})"},{"line_number":381,"context_line":"                    return"},{"line_number":382,"context_line":"            except Exception as e:"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa7ab95a_3bfde93a","line":379,"range":{"start_line":379,"start_character":24,"end_line":379,"end_character":29},"updated":"2016-08-26 14:48:19.000000000","message":"I would raise the original exception instead","commit_id":"05b8fe231580156a305b0124df728a6f11e418c9"},{"author":{"_account_id":14680,"name":"Rick Aulino","email":"rick.aulino@hpe.com","username":"ricka"},"change_message_id":"bf8fa852377520b35cbfb35131e781e9236de4d4","unresolved":false,"context_lines":[{"line_number":376,"context_line":"                    # Conflict error, retry with new version of doc."},{"line_number":377,"context_line":"                    pass"},{"line_number":378,"context_line":"                else:"},{"line_number":379,"context_line":"                    LOG.error(_LE(\u0027Error adding security group rule %(id)s\u0027) %"},{"line_number":380,"context_line":"                              {\u0027id\u0027: group_id})"},{"line_number":381,"context_line":"                    return"},{"line_number":382,"context_line":"            except Exception as e:"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa7ab95a_7414d043","line":379,"range":{"start_line":379,"start_character":24,"end_line":379,"end_character":29},"in_reply_to":"fa7ab95a_3bfde93a","updated":"2016-08-26 19:28:16.000000000","message":"This (and the exception catching) below was part of the original intent to catch/react to all ES exceptions in an appropriate manner. Since we have split that work into a different patch, I\u0027ll remove these exception catches.","commit_id":"05b8fe231580156a305b0124df728a6f11e418c9"},{"author":{"_account_id":10063,"name":"Steve McLellan","email":"steven.j.mclellan@gmail.com","username":"sjmc7"},"change_message_id":"b12f7f4520c4bffc9bd26c03df24d55d5f0c67c8","unresolved":false,"context_lines":[{"line_number":379,"context_line":"                    LOG.error(_LE(\u0027Error adding security group rule %(id)s\u0027) %"},{"line_number":380,"context_line":"                              {\u0027id\u0027: group_id})"},{"line_number":381,"context_line":"                    return"},{"line_number":382,"context_line":"            except Exception as e:"},{"line_number":383,"context_line":"                LOG.error(_LE(\u0027Error adding security group rule %(id)s\u0027) %"},{"line_number":384,"context_line":"                          {\u0027id\u0027: group_id})"},{"line_number":385,"context_line":"                return"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa7ab95a_bbe8197e","line":382,"range":{"start_line":382,"start_character":19,"end_line":382,"end_character":28},"updated":"2016-08-26 14:48:19.000000000","message":"Is this necessary? In general I think in general we let uncaught exceptions bubble up","commit_id":"05b8fe231580156a305b0124df728a6f11e418c9"},{"author":{"_account_id":14680,"name":"Rick Aulino","email":"rick.aulino@hpe.com","username":"ricka"},"change_message_id":"bf8fa852377520b35cbfb35131e781e9236de4d4","unresolved":false,"context_lines":[{"line_number":379,"context_line":"                    LOG.error(_LE(\u0027Error adding security group rule %(id)s\u0027) %"},{"line_number":380,"context_line":"                              {\u0027id\u0027: group_id})"},{"line_number":381,"context_line":"                    return"},{"line_number":382,"context_line":"            except Exception as e:"},{"line_number":383,"context_line":"                LOG.error(_LE(\u0027Error adding security group rule %(id)s\u0027) %"},{"line_number":384,"context_line":"                          {\u0027id\u0027: group_id})"},{"line_number":385,"context_line":"                return"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa7ab95a_940f942d","line":382,"range":{"start_line":382,"start_character":19,"end_line":382,"end_character":28},"in_reply_to":"fa7ab95a_bbe8197e","updated":"2016-08-26 19:28:16.000000000","message":"Removed as per the rationale above.","commit_id":"05b8fe231580156a305b0124df728a6f11e418c9"},{"author":{"_account_id":10063,"name":"Steve McLellan","email":"steven.j.mclellan@gmail.com","username":"sjmc7"},"change_message_id":"b12f7f4520c4bffc9bd26c03df24d55d5f0c67c8","unresolved":false,"context_lines":[{"line_number":385,"context_line":"                return"},{"line_number":386,"context_line":""},{"line_number":387,"context_line":"        if attempts \u003d\u003d (retries - 1):"},{"line_number":388,"context_line":"            LOG.error(_LE(\u0027Error adding security group rule %(id)s\u0027) %"},{"line_number":389,"context_line":"                      {\u0027id\u0027: group_id})"},{"line_number":390,"context_line":""},{"line_number":391,"context_line":"    def delete_rule(self, payload, timestamp):"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa7ab95a_3bbfa9fc","line":388,"range":{"start_line":388,"start_character":27,"end_line":388,"end_character":32},"updated":"2016-08-26 14:48:19.000000000","message":"It\u0027d be good to note that this is because of too many retries due to conflicts","commit_id":"05b8fe231580156a305b0124df728a6f11e418c9"},{"author":{"_account_id":14680,"name":"Rick Aulino","email":"rick.aulino@hpe.com","username":"ricka"},"change_message_id":"bf8fa852377520b35cbfb35131e781e9236de4d4","unresolved":false,"context_lines":[{"line_number":385,"context_line":"                return"},{"line_number":386,"context_line":""},{"line_number":387,"context_line":"        if attempts \u003d\u003d (retries - 1):"},{"line_number":388,"context_line":"            LOG.error(_LE(\u0027Error adding security group rule %(id)s\u0027) %"},{"line_number":389,"context_line":"                      {\u0027id\u0027: group_id})"},{"line_number":390,"context_line":""},{"line_number":391,"context_line":"    def delete_rule(self, payload, timestamp):"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa7ab95a_d4191c7b","line":388,"range":{"start_line":388,"start_character":27,"end_line":388,"end_character":32},"in_reply_to":"fa7ab95a_3bbfa9fc","updated":"2016-08-26 19:28:16.000000000","message":"Noted. And it is good!","commit_id":"05b8fe231580156a305b0124df728a6f11e418c9"},{"author":{"_account_id":14680,"name":"Rick Aulino","email":"rick.aulino@hpe.com","username":"ricka"},"change_message_id":"bf8fa852377520b35cbfb35131e781e9236de4d4","unresolved":false,"context_lines":[{"line_number":431,"context_line":"                    # Conflict error, retry with new version of doc."},{"line_number":432,"context_line":"                    pass"},{"line_number":433,"context_line":"                else:"},{"line_number":434,"context_line":"                    LOG.error(_LE(\u0027Error deleting security group rule %(id)s\u0027)"},{"line_number":435,"context_line":"                              % {\u0027id\u0027: rule_id})"},{"line_number":436,"context_line":"                    return"},{"line_number":437,"context_line":"            except Exception as e:"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa7ab95a_34588832","line":434,"updated":"2016-08-26 19:28:16.000000000","message":"Same changes as above. Hopefully all references to \"create\" were changed to \"delete\" in the copy/paste :)","commit_id":"05b8fe231580156a305b0124df728a6f11e418c9"},{"author":{"_account_id":10063,"name":"Steve McLellan","email":"steven.j.mclellan@gmail.com","username":"sjmc7"},"change_message_id":"c315dce77bbc470ba6400fe924c74f65bed093c7","unresolved":false,"context_lines":[{"line_number":403,"context_line":""},{"line_number":404,"context_line":"        # Read, modify, write of an existing security group."},{"line_number":405,"context_line":"        for attempts in range(retries):"},{"line_number":406,"context_line":"            doc \u003d self.get_doc_by_nested_field("},{"line_number":407,"context_line":"                \"security_group_rules\", \"id\", rule_id, version\u003dTrue)"},{"line_number":408,"context_line":""},{"line_number":409,"context_line":"            if not doc:"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a89bdaa_65d34739","line":406,"range":{"start_line":406,"start_character":23,"end_line":406,"end_character":46},"updated":"2016-09-05 15:29:35.000000000","message":"I think the logic needs to change here. I\u0027ve tested this by deleting several rules at once, and when one event handler fails I believe it\u0027s because it\u0027s running the same query 20 times but the ES indexer hasn\u0027t yet reindexed the results. A sleep would fix it but be ugly; my preferred solution would be to use the results of the initial query to get the sec group id and then use get_document which will always return the latest version.","commit_id":"740a30bcb4150bc597d19b35020d2e91a1d84c80"},{"author":{"_account_id":14680,"name":"Rick Aulino","email":"rick.aulino@hpe.com","username":"ricka"},"change_message_id":"62cf670e5d93c715262395733f23f0d45ffb063a","unresolved":false,"context_lines":[{"line_number":403,"context_line":""},{"line_number":404,"context_line":"        # Read, modify, write of an existing security group."},{"line_number":405,"context_line":"        for attempts in range(retries):"},{"line_number":406,"context_line":"            doc \u003d self.get_doc_by_nested_field("},{"line_number":407,"context_line":"                \"security_group_rules\", \"id\", rule_id, version\u003dTrue)"},{"line_number":408,"context_line":""},{"line_number":409,"context_line":"            if not doc:"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a89bdaa_239d72bc","line":406,"range":{"start_line":406,"start_character":23,"end_line":406,"end_character":46},"in_reply_to":"9a89bdaa_65d34739","updated":"2016-09-06 23:17:10.000000000","message":"Done.","commit_id":"740a30bcb4150bc597d19b35020d2e91a1d84c80"},{"author":{"_account_id":10063,"name":"Steve McLellan","email":"steven.j.mclellan@gmail.com","username":"sjmc7"},"change_message_id":"c315dce77bbc470ba6400fe924c74f65bed093c7","unresolved":false,"context_lines":[{"line_number":422,"context_line":"                break"},{"line_number":423,"context_line":"            except helpers.BulkIndexError as e:"},{"line_number":424,"context_line":"                if e.errors[0][\u0027index\u0027][\u0027status\u0027] \u003d\u003d 409:"},{"line_number":425,"context_line":"                    # Conflict error, retry with new version of doc."},{"line_number":426,"context_line":"                    pass"},{"line_number":427,"context_line":"                else:"},{"line_number":428,"context_line":"                    raise"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a89bdaa_25f3ff9f","line":425,"range":{"start_line":425,"start_character":31,"end_line":425,"end_character":36},"updated":"2016-09-05 15:29:35.000000000","message":"If you add a debug line here you can see the not-yet-reindexed problem","commit_id":"740a30bcb4150bc597d19b35020d2e91a1d84c80"},{"author":{"_account_id":10063,"name":"Steve McLellan","email":"steven.j.mclellan@gmail.com","username":"sjmc7"},"change_message_id":"00e4cab3c04f0f0c09d4f6c175a3fd72335cc5eb","unresolved":false,"context_lines":[{"line_number":409,"context_line":"        # directly by the ID which will always return the latest version."},{"line_number":410,"context_line":"        orig_doc \u003d self.get_doc_by_nested_field("},{"line_number":411,"context_line":"            \"security_group_rules\", \"id\", rule_id, version\u003dTrue)"},{"line_number":412,"context_line":"        if not orig_doc:"},{"line_number":413,"context_line":"            return"},{"line_number":414,"context_line":"        doc_id \u003d orig_doc[\u0027hits\u0027][\u0027hits\u0027][0][\u0027_id\u0027]"},{"line_number":415,"context_line":"        for attempts in range(retries):"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a89bdaa_8af2186e","line":412,"range":{"start_line":412,"start_character":11,"end_line":412,"end_character":14},"updated":"2016-09-09 18:43:45.000000000","message":"This will expose a race condition if a group hasn\u0027t become visible before a user starts adding rules to it; in practice i doubt it\u0027ll ever be a problem and in any case a rapid loop would be unlikely to catch it; you\u0027d want some kind of back-off","commit_id":"ef33ca32430c3e1519c45a9b04ba955c4eab398e"},{"author":{"_account_id":14680,"name":"Rick Aulino","email":"rick.aulino@hpe.com","username":"ricka"},"change_message_id":"10fec299a29487e1616245399ca7f95395fc792b","unresolved":false,"context_lines":[{"line_number":409,"context_line":"        # directly by the ID which will always return the latest version."},{"line_number":410,"context_line":"        orig_doc \u003d self.get_doc_by_nested_field("},{"line_number":411,"context_line":"            \"security_group_rules\", \"id\", rule_id, version\u003dTrue)"},{"line_number":412,"context_line":"        if not orig_doc:"},{"line_number":413,"context_line":"            return"},{"line_number":414,"context_line":"        doc_id \u003d orig_doc[\u0027hits\u0027][\u0027hits\u0027][0][\u0027_id\u0027]"},{"line_number":415,"context_line":"        for attempts in range(retries):"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a89bdaa_23e63b15","line":412,"range":{"start_line":412,"start_character":11,"end_line":412,"end_character":14},"in_reply_to":"9a89bdaa_8af2186e","updated":"2016-09-13 00:05:08.000000000","message":"Made the retry less rapid.","commit_id":"ef33ca32430c3e1519c45a9b04ba955c4eab398e"},{"author":{"_account_id":10063,"name":"Steve McLellan","email":"steven.j.mclellan@gmail.com","username":"sjmc7"},"change_message_id":"00e4cab3c04f0f0c09d4f6c175a3fd72335cc5eb","unresolved":false,"context_lines":[{"line_number":413,"context_line":"            return"},{"line_number":414,"context_line":"        doc_id \u003d orig_doc[\u0027hits\u0027][\u0027hits\u0027][0][\u0027_id\u0027]"},{"line_number":415,"context_line":"        for attempts in range(retries):"},{"line_number":416,"context_line":"            doc \u003d self.index_helper.get_document(doc_id)"},{"line_number":417,"context_line":""},{"line_number":418,"context_line":"            if not doc:"},{"line_number":419,"context_line":"                return"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a89bdaa_eaaf2429","line":416,"range":{"start_line":416,"start_character":23,"end_line":416,"end_character":35},"updated":"2016-09-09 18:43:45.000000000","message":"You already have the document in orig_doc, no? This can move to either inside the 409 handler or at the end of the loop if orig_doc is renamed \u0027doc\u0027 throughout","commit_id":"ef33ca32430c3e1519c45a9b04ba955c4eab398e"},{"author":{"_account_id":14680,"name":"Rick Aulino","email":"rick.aulino@hpe.com","username":"ricka"},"change_message_id":"10fec299a29487e1616245399ca7f95395fc792b","unresolved":false,"context_lines":[{"line_number":413,"context_line":"            return"},{"line_number":414,"context_line":"        doc_id \u003d orig_doc[\u0027hits\u0027][\u0027hits\u0027][0][\u0027_id\u0027]"},{"line_number":415,"context_line":"        for attempts in range(retries):"},{"line_number":416,"context_line":"            doc \u003d self.index_helper.get_document(doc_id)"},{"line_number":417,"context_line":""},{"line_number":418,"context_line":"            if not doc:"},{"line_number":419,"context_line":"                return"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a89bdaa_033cd7c6","line":416,"range":{"start_line":416,"start_character":23,"end_line":416,"end_character":35},"in_reply_to":"9a89bdaa_eaaf2429","updated":"2016-09-13 00:05:08.000000000","message":"Sort of. The call to get_doc_by_nested_field is only to get the document ID. The document ID is what we want to use int he loop. The return value of get_doc_by_nested_field() is different from the return value of get_document(). One is the entire Elasticsearch results (i.e. [\u0027hits\u0027][\u0027hits\u0027]...). The other is the truncated [\u0027_source\u0027] results. To make the loop simpler and not trying to worry about how to parse the return value we are using the results from get_document() only in the loop. It results in a single \"extra\" call, but that is in the noise.","commit_id":"ef33ca32430c3e1519c45a9b04ba955c4eab398e"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"af7ad62100e5eb71a2cc300acc8c7a7c510e32b6","unresolved":false,"context_lines":[{"line_number":351,"context_line":"        # NB: Most of the looping logic is the same as in \"delete_rule\"."},{"line_number":352,"context_line":"        #     The read/modify the ES document is different. If the logic"},{"line_number":353,"context_line":"        #     changes, please make the changes there."},{"line_number":354,"context_line":"        retries \u003d 20"},{"line_number":355,"context_line":"        group_id \u003d payload[\u0027security_group_rule\u0027][\u0027security_group_id\u0027]"},{"line_number":356,"context_line":"        LOG.debug(\"Updating security group rule information for %s\", group_id)"},{"line_number":357,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"9a89bdaa_1a142d4b","line":354,"range":{"start_line":354,"start_character":8,"end_line":354,"end_character":20},"updated":"2016-09-13 07:04:47.000000000","message":"make this a config option and use for other porential race conditions?","commit_id":"f435a19d5ec14a13d0c8253cd71d8823b67bf4cf"},{"author":{"_account_id":10063,"name":"Steve McLellan","email":"steven.j.mclellan@gmail.com","username":"sjmc7"},"change_message_id":"13e0e008101c1c8595a6b0dca09c320cde1373d8","unresolved":false,"context_lines":[{"line_number":351,"context_line":"        # NB: Most of the looping logic is the same as in \"delete_rule\"."},{"line_number":352,"context_line":"        #     The read/modify the ES document is different. If the logic"},{"line_number":353,"context_line":"        #     changes, please make the changes there."},{"line_number":354,"context_line":"        retries \u003d 20"},{"line_number":355,"context_line":"        group_id \u003d payload[\u0027security_group_rule\u0027][\u0027security_group_id\u0027]"},{"line_number":356,"context_line":"        LOG.debug(\"Updating security group rule information for %s\", group_id)"},{"line_number":357,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"7a8ec9b2_9a24b3c5","line":354,"range":{"start_line":354,"start_character":8,"end_line":354,"end_character":20},"in_reply_to":"9a89bdaa_1a142d4b","updated":"2016-09-16 14:30:58.000000000","message":"I\u0027m ok with this not being a config setting. It\u0027s likely that if  this were reimplemented elsewhere the number of retries would differ - security groups may easily have several rules arriving at once (though 20 sounds a bit extreme) but another type might not expect so many changes and so would use a smaller number. Also if we repeat this pattern I want it to be a decorator accepting a \u0027number_retries\u0027 parameter.\n\nI would be in favor of making this a constant in this file though.","commit_id":"f435a19d5ec14a13d0c8253cd71d8823b67bf4cf"},{"author":{"_account_id":10063,"name":"Steve McLellan","email":"steven.j.mclellan@gmail.com","username":"sjmc7"},"change_message_id":"13e0e008101c1c8595a6b0dca09c320cde1373d8","unresolved":false,"context_lines":[{"line_number":357,"context_line":""},{"line_number":358,"context_line":"        for attempts in range(retries):"},{"line_number":359,"context_line":"            # Read, modify, write of an existing security group."},{"line_number":360,"context_line":"            doc \u003d self.index_helper.get_document(group_id)"},{"line_number":361,"context_line":""},{"line_number":362,"context_line":"            if not doc:"},{"line_number":363,"context_line":"                return"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a8ec9b2_f5894827","line":360,"range":{"start_line":360,"start_character":23,"end_line":360,"end_character":35},"updated":"2016-09-16 14:30:58.000000000","message":"Still the same comment about inefficiency. It\u0027s guaranteed in every case we\u0027ll repeat the work bey retrieving a second time - i would be fine moving this line into the body of the 409 error handler.","commit_id":"f435a19d5ec14a13d0c8253cd71d8823b67bf4cf"},{"author":{"_account_id":10063,"name":"Steve McLellan","email":"steven.j.mclellan@gmail.com","username":"sjmc7"},"change_message_id":"13e0e008101c1c8595a6b0dca09c320cde1373d8","unresolved":false,"context_lines":[{"line_number":384,"context_line":"                          \u0027 Too many retries\u0027) % {\u0027id\u0027: group_id})"},{"line_number":385,"context_line":""},{"line_number":386,"context_line":"    def delete_rule(self, payload, timestamp):"},{"line_number":387,"context_line":"        # The issue here is that the notification is not complete."},{"line_number":388,"context_line":"        # We have only a single rule that needs to be deleted from an"},{"line_number":389,"context_line":"        # existing group. A major issue is that we may be updating"},{"line_number":390,"context_line":"        # the ES document while other workers are modifying the rules"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a8ec9b2_d5b9ac16","line":387,"range":{"start_line":387,"start_character":36,"end_line":387,"end_character":37},"updated":"2016-09-16 14:30:58.000000000","message":"We don\u0027t need to reproduce this entire comment. Maybe \u0027see note above\u0027","commit_id":"f435a19d5ec14a13d0c8253cd71d8823b67bf4cf"},{"author":{"_account_id":10063,"name":"Steve McLellan","email":"steven.j.mclellan@gmail.com","username":"sjmc7"},"change_message_id":"13e0e008101c1c8595a6b0dca09c320cde1373d8","unresolved":false,"context_lines":[{"line_number":414,"context_line":"            return"},{"line_number":415,"context_line":"        doc_id \u003d orig_doc[\u0027hits\u0027][\u0027hits\u0027][0][\u0027_id\u0027]"},{"line_number":416,"context_line":"        for attempts in range(retries):"},{"line_number":417,"context_line":"            doc \u003d self.index_helper.get_document(doc_id)"},{"line_number":418,"context_line":""},{"line_number":419,"context_line":"            if not doc:"},{"line_number":420,"context_line":"                return"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a8ec9b2_b5ab4074","line":417,"range":{"start_line":417,"start_character":16,"end_line":417,"end_character":17},"updated":"2016-09-16 14:30:58.000000000","message":"Same comment as in create_or_update_rule","commit_id":"f435a19d5ec14a13d0c8253cd71d8823b67bf4cf"},{"author":{"_account_id":10063,"name":"Steve McLellan","email":"steven.j.mclellan@gmail.com","username":"sjmc7"},"change_message_id":"13e0e008101c1c8595a6b0dca09c320cde1373d8","unresolved":false,"context_lines":[{"line_number":433,"context_line":"            except helpers.BulkIndexError as e:"},{"line_number":434,"context_line":"                if e.errors[0][\u0027index\u0027][\u0027status\u0027] \u003d\u003d 409:"},{"line_number":435,"context_line":"                    # Conflict. Retry with new version, but not too fast."},{"line_number":436,"context_line":"                    sleep(0.5)"},{"line_number":437,"context_line":""},{"line_number":438,"context_line":"                    pass"},{"line_number":439,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a8ec9b2_55011c69","line":436,"range":{"start_line":436,"start_character":20,"end_line":436,"end_character":25},"updated":"2016-09-16 14:30:58.000000000","message":"There\u0027s a sleep here but not in create_or_update_rule; makes sense to be consistent. I can\u0027t decide whether the sleep() is worthwhile since it will affect all processes equally; if 15 rules come in at once then the sleep won\u0027t help resolve anything, just make the race take longer. I\u0027m a little hesitant at building in a potential 10 second wait","commit_id":"f435a19d5ec14a13d0c8253cd71d8823b67bf4cf"},{"author":{"_account_id":4428,"name":"Liyingjun","email":"yinjalee@163.com","username":"liyingjun"},"change_message_id":"3eaddf40519c5f6fd18ae00ebee182042a0147b6","unresolved":false,"context_lines":[{"line_number":403,"context_line":"        doc_id \u003d orig_doc[\u0027hits\u0027][\u0027hits\u0027][0][\u0027_id\u0027]"},{"line_number":404,"context_line":"        doc \u003d orig_doc[\u0027hits\u0027][\u0027hits\u0027][0]"},{"line_number":405,"context_line":"        for attempts in range(SECGROUP_RETRIES):"},{"line_number":406,"context_line":"            if not doc:"},{"line_number":407,"context_line":"                return"},{"line_number":408,"context_line":"            body \u003d doc[\u0027_source\u0027]"},{"line_number":409,"context_line":"            if not body or field not in body:"},{"line_number":410,"context_line":"                return"}],"source_content_type":"text/x-python","patch_set":9,"id":"7a8ec9b2_f45d1c7a","line":407,"range":{"start_line":406,"start_character":0,"end_line":407,"end_character":22},"updated":"2016-09-21 00:20:17.000000000","message":"These two lines are not needed, if not doc, error will be raised in L403","commit_id":"721eb27a4ebe35fc1b0c57380c64ee460b8a17ca"},{"author":{"_account_id":14680,"name":"Rick Aulino","email":"rick.aulino@hpe.com","username":"ricka"},"change_message_id":"95c374d0d5f1f0365d00d76a7abde44a8c02234b","unresolved":false,"context_lines":[{"line_number":403,"context_line":"        doc_id \u003d orig_doc[\u0027hits\u0027][\u0027hits\u0027][0][\u0027_id\u0027]"},{"line_number":404,"context_line":"        doc \u003d orig_doc[\u0027hits\u0027][\u0027hits\u0027][0]"},{"line_number":405,"context_line":"        for attempts in range(SECGROUP_RETRIES):"},{"line_number":406,"context_line":"            if not doc:"},{"line_number":407,"context_line":"                return"},{"line_number":408,"context_line":"            body \u003d doc[\u0027_source\u0027]"},{"line_number":409,"context_line":"            if not body or field not in body:"},{"line_number":410,"context_line":"                return"}],"source_content_type":"text/x-python","patch_set":9,"id":"7a8ec9b2_72de4712","line":407,"range":{"start_line":406,"start_character":0,"end_line":407,"end_character":22},"in_reply_to":"7a8ec9b2_6834c5fa","updated":"2016-09-21 16:13:25.000000000","message":"Actually, this test is related to line 423 below. If you prefer, I can move it down there. Originally the loop had line 423 above this check.","commit_id":"721eb27a4ebe35fc1b0c57380c64ee460b8a17ca"},{"author":{"_account_id":10063,"name":"Steve McLellan","email":"steven.j.mclellan@gmail.com","username":"sjmc7"},"change_message_id":"2b3d7fe233c1627ded761743b46905e55efc8170","unresolved":false,"context_lines":[{"line_number":403,"context_line":"        doc_id \u003d orig_doc[\u0027hits\u0027][\u0027hits\u0027][0][\u0027_id\u0027]"},{"line_number":404,"context_line":"        doc \u003d orig_doc[\u0027hits\u0027][\u0027hits\u0027][0]"},{"line_number":405,"context_line":"        for attempts in range(SECGROUP_RETRIES):"},{"line_number":406,"context_line":"            if not doc:"},{"line_number":407,"context_line":"                return"},{"line_number":408,"context_line":"            body \u003d doc[\u0027_source\u0027]"},{"line_number":409,"context_line":"            if not body or field not in body:"},{"line_number":410,"context_line":"                return"}],"source_content_type":"text/x-python","patch_set":9,"id":"7a8ec9b2_92503334","line":407,"range":{"start_line":406,"start_character":0,"end_line":407,"end_character":22},"in_reply_to":"7a8ec9b2_72de4712","updated":"2016-09-21 16:17:28.000000000","message":"Yeah, maybe makes sense to move it down there for clarity","commit_id":"721eb27a4ebe35fc1b0c57380c64ee460b8a17ca"},{"author":{"_account_id":14680,"name":"Rick Aulino","email":"rick.aulino@hpe.com","username":"ricka"},"change_message_id":"25899cb43e07012d8ce7186b9abdcb6c6d9aa365","unresolved":false,"context_lines":[{"line_number":403,"context_line":"        doc_id \u003d orig_doc[\u0027hits\u0027][\u0027hits\u0027][0][\u0027_id\u0027]"},{"line_number":404,"context_line":"        doc \u003d orig_doc[\u0027hits\u0027][\u0027hits\u0027][0]"},{"line_number":405,"context_line":"        for attempts in range(SECGROUP_RETRIES):"},{"line_number":406,"context_line":"            if not doc:"},{"line_number":407,"context_line":"                return"},{"line_number":408,"context_line":"            body \u003d doc[\u0027_source\u0027]"},{"line_number":409,"context_line":"            if not body or field not in body:"},{"line_number":410,"context_line":"                return"}],"source_content_type":"text/x-python","patch_set":9,"id":"7a8ec9b2_58496930","line":407,"range":{"start_line":406,"start_character":0,"end_line":407,"end_character":22},"in_reply_to":"7a8ec9b2_92503334","updated":"2016-09-21 17:38:40.000000000","message":"Three folks have spoken. Done.","commit_id":"721eb27a4ebe35fc1b0c57380c64ee460b8a17ca"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"476d33381c01e832ad89da8e76a4c9c8afccb397","unresolved":false,"context_lines":[{"line_number":403,"context_line":"        doc_id \u003d orig_doc[\u0027hits\u0027][\u0027hits\u0027][0][\u0027_id\u0027]"},{"line_number":404,"context_line":"        doc \u003d orig_doc[\u0027hits\u0027][\u0027hits\u0027][0]"},{"line_number":405,"context_line":"        for attempts in range(SECGROUP_RETRIES):"},{"line_number":406,"context_line":"            if not doc:"},{"line_number":407,"context_line":"                return"},{"line_number":408,"context_line":"            body \u003d doc[\u0027_source\u0027]"},{"line_number":409,"context_line":"            if not body or field not in body:"},{"line_number":410,"context_line":"                return"}],"source_content_type":"text/x-python","patch_set":9,"id":"7a8ec9b2_6834c5fa","line":407,"range":{"start_line":406,"start_character":0,"end_line":407,"end_character":22},"in_reply_to":"7a8ec9b2_f45d1c7a","updated":"2016-09-21 06:25:48.000000000","message":"+1","commit_id":"721eb27a4ebe35fc1b0c57380c64ee460b8a17ca"}]}
