)]}'
{"neutron/plugins/linuxbridge/agent/linuxbridge_neutron_agent.py":[{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"e1d8b167da7c4d9afb82e9ae0b0b288cc959a09e","unresolved":false,"context_lines":[{"line_number":915,"context_line":"            start \u003d time.time()"},{"line_number":916,"context_line":"            if sync:"},{"line_number":917,"context_line":"                LOG.info(_(\"Agent out of sync with plugin!\"))"},{"line_number":918,"context_line":"                seen_devices |\u003d device_info[\u0027current\u0027].copy()"},{"line_number":919,"context_line":"                registered_devices.clear()"},{"line_number":920,"context_line":"                sync \u003d False"},{"line_number":921,"context_line":"            # Save updated devices dict to perform rollback in case"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAWH%2F9TH0%3D","line":918,"updated":"2014-04-07 09:43:27.000000000","message":"not sure there is a need to copy here, as you\u0027re using |\u003d","commit_id":"0fa1c8831fee905ae351d80b26e3ae9b91975104"},{"author":{"_account_id":2733,"name":"Darragh O\u0027Reilly","email":"doreilly@suse.com","username":"darragh-oreilly"},"change_message_id":"963618fe852808d2eda48925fe06f6478bbb9cb0","unresolved":false,"context_lines":[{"line_number":915,"context_line":"            start \u003d time.time()"},{"line_number":916,"context_line":"            if sync:"},{"line_number":917,"context_line":"                LOG.info(_(\"Agent out of sync with plugin!\"))"},{"line_number":918,"context_line":"                seen_devices |\u003d device_info[\u0027current\u0027].copy()"},{"line_number":919,"context_line":"                registered_devices.clear()"},{"line_number":920,"context_line":"                sync \u003d False"},{"line_number":921,"context_line":"            # Save updated devices dict to perform rollback in case"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAWn%2F%2F9sg%3D","line":918,"in_reply_to":"AAAAWH%2F9TH0%3D","updated":"2014-04-08 08:27:23.000000000","message":"you are right, I\u0027ll take it out.","commit_id":"0fa1c8831fee905ae351d80b26e3ae9b91975104"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"e1d8b167da7c4d9afb82e9ae0b0b288cc959a09e","unresolved":false,"context_lines":[{"line_number":932,"context_line":"                    LOG.debug(_(\"Agent loop found changes! %s\"), device_info)"},{"line_number":933,"context_line":"                    sync \u003d self.process_network_devices(device_info)"},{"line_number":934,"context_line":"                    if not sync:"},{"line_number":935,"context_line":"                        # We won\u0027t be here if there was an exception either."},{"line_number":936,"context_line":"                        registered_devices \u003d device_info[\u0027current\u0027].copy()"},{"line_number":937,"context_line":"                        seen_devices \u003d device_info[\u0027current\u0027].copy()"},{"line_number":938,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAWH%2F9TG0%3D","line":935,"updated":"2014-04-07 09:43:27.000000000","message":"grammar nazi comment - why the \u0027either\u0027 at the end of the sentence?","commit_id":"0fa1c8831fee905ae351d80b26e3ae9b91975104"},{"author":{"_account_id":2733,"name":"Darragh O\u0027Reilly","email":"doreilly@suse.com","username":"darragh-oreilly"},"change_message_id":"963618fe852808d2eda48925fe06f6478bbb9cb0","unresolved":false,"context_lines":[{"line_number":932,"context_line":"                    LOG.debug(_(\"Agent loop found changes! %s\"), device_info)"},{"line_number":933,"context_line":"                    sync \u003d self.process_network_devices(device_info)"},{"line_number":934,"context_line":"                    if not sync:"},{"line_number":935,"context_line":"                        # We won\u0027t be here if there was an exception either."},{"line_number":936,"context_line":"                        registered_devices \u003d device_info[\u0027current\u0027].copy()"},{"line_number":937,"context_line":"                        seen_devices \u003d device_info[\u0027current\u0027].copy()"},{"line_number":938,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAWn%2F%2F9n4%3D","line":935,"in_reply_to":"AAAAWH%2F9TG0%3D","updated":"2014-04-08 08:27:23.000000000","message":":) so I was just trying to highlight that we won\u0027t be here if process_network_devices causes an exception, as well as when it returns True. But the the latter is obvious. I\u0027ll take it out.","commit_id":"0fa1c8831fee905ae351d80b26e3ae9b91975104"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"e1d8b167da7c4d9afb82e9ae0b0b288cc959a09e","unresolved":false,"context_lines":[{"line_number":934,"context_line":"                    if not sync:"},{"line_number":935,"context_line":"                        # We won\u0027t be here if there was an exception either."},{"line_number":936,"context_line":"                        registered_devices \u003d device_info[\u0027current\u0027].copy()"},{"line_number":937,"context_line":"                        seen_devices \u003d device_info[\u0027current\u0027].copy()"},{"line_number":938,"context_line":"            except Exception:"},{"line_number":939,"context_line":"                LOG.exception(_(\"Error in agent loop. Devices info: %s\"),"},{"line_number":940,"context_line":"                              device_info)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAWH%2F9THk%3D","line":937,"updated":"2014-04-07 09:43:27.000000000","message":"I am not sure there is a need for copy here either","commit_id":"0fa1c8831fee905ae351d80b26e3ae9b91975104"},{"author":{"_account_id":2733,"name":"Darragh O\u0027Reilly","email":"doreilly@suse.com","username":"darragh-oreilly"},"change_message_id":"963618fe852808d2eda48925fe06f6478bbb9cb0","unresolved":false,"context_lines":[{"line_number":934,"context_line":"                    if not sync:"},{"line_number":935,"context_line":"                        # We won\u0027t be here if there was an exception either."},{"line_number":936,"context_line":"                        registered_devices \u003d device_info[\u0027current\u0027].copy()"},{"line_number":937,"context_line":"                        seen_devices \u003d device_info[\u0027current\u0027].copy()"},{"line_number":938,"context_line":"            except Exception:"},{"line_number":939,"context_line":"                LOG.exception(_(\"Error in agent loop. Devices info: %s\"),"},{"line_number":940,"context_line":"                              device_info)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAWn%2F%2F9jQ%3D","line":937,"in_reply_to":"AAAAWH%2F9THk%3D","updated":"2014-04-08 08:27:23.000000000","message":"yeah there is no need. I\u0027ll need to simplify the unit test, as mock checks the contents of the set pointed to by device_info[\u0027current\u0027] at the end of the test, but L918 has changed it by that time.","commit_id":"0fa1c8831fee905ae351d80b26e3ae9b91975104"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"5a4519cfb79814d45e36156535dd9858307e1436","unresolved":false,"context_lines":[{"line_number":933,"context_line":"        curr_devices \u003d self.br_mgr.get_tap_devices()"},{"line_number":934,"context_line":"        device_info \u003d {}"},{"line_number":935,"context_line":"        device_info[\u0027current\u0027] \u003d curr_devices"},{"line_number":936,"context_line":"        device_info[\u0027added\u0027] \u003d curr_devices - registered_devices"},{"line_number":937,"context_line":"        # we don\u0027t want to process updates for devices that don\u0027t exist"},{"line_number":938,"context_line":"        device_info[\u0027updated\u0027] \u003d updated_devices \u0026 curr_devices"},{"line_number":939,"context_line":"        # we need to clean up after devices are removed"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAW3%2F%2FlUQ%3D","line":936,"updated":"2014-04-12 00:28:45.000000000","message":"Could seen_devices be used here as well?  That would eliminate the need for two sets and could simplify this patch a bit.\n\nI might be missing something.","commit_id":"7e42aec19b1bf4a14849deb4c96949051ee723ae"},{"author":{"_account_id":2733,"name":"Darragh O\u0027Reilly","email":"doreilly@suse.com","username":"darragh-oreilly"},"change_message_id":"ef8fb2c8d2b58e76ca294e9b9568e76c1f3b7c5f","unresolved":false,"context_lines":[{"line_number":933,"context_line":"        curr_devices \u003d self.br_mgr.get_tap_devices()"},{"line_number":934,"context_line":"        device_info \u003d {}"},{"line_number":935,"context_line":"        device_info[\u0027current\u0027] \u003d curr_devices"},{"line_number":936,"context_line":"        device_info[\u0027added\u0027] \u003d curr_devices - registered_devices"},{"line_number":937,"context_line":"        # we don\u0027t want to process updates for devices that don\u0027t exist"},{"line_number":938,"context_line":"        device_info[\u0027updated\u0027] \u003d updated_devices \u0026 curr_devices"},{"line_number":939,"context_line":"        # we need to clean up after devices are removed"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAXH%2F%2Fz1A%3D","line":936,"in_reply_to":"AAAAW3%2F%2FlUQ%3D","updated":"2014-04-17 09:32:08.000000000","message":"Sorry Carl, I\u0027m not sure what the question is. I think it is: can seen_devices be used instead of registered_devices on L936? If so, then no it can\u0027t, because in the case where process_network_devices() raised an exception, or returned True, registered_devices will have been reset to an empty set on L963, but seen_devices will contain previously registered and unregistered devices.\n\nI\u0027ll think about ways to simplify this area, as it\u0027s a bit hard to follow with the logic spread over many places. One thing would be to move L978-980 up and insert after L964.","commit_id":"7e42aec19b1bf4a14849deb4c96949051ee723ae"},{"author":{"_account_id":2733,"name":"Darragh O\u0027Reilly","email":"doreilly@suse.com","username":"darragh-oreilly"},"change_message_id":"bb696d5816ec58cd1675f64a6753c1f6ec3adde3","unresolved":false,"context_lines":[{"line_number":933,"context_line":"        curr_devices \u003d self.br_mgr.get_tap_devices()"},{"line_number":934,"context_line":"        device_info \u003d {}"},{"line_number":935,"context_line":"        device_info[\u0027current\u0027] \u003d curr_devices"},{"line_number":936,"context_line":"        device_info[\u0027added\u0027] \u003d curr_devices - registered_devices"},{"line_number":937,"context_line":"        # we don\u0027t want to process updates for devices that don\u0027t exist"},{"line_number":938,"context_line":"        device_info[\u0027updated\u0027] \u003d updated_devices \u0026 curr_devices"},{"line_number":939,"context_line":"        # we need to clean up after devices are removed"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAXH%2F%2FqxA%3D","line":936,"in_reply_to":"AAAAXH%2F%2Fu2U%3D","updated":"2014-04-18 12:01:56.000000000","message":"So registered_devices is being cleared on L963 because that was the strategy prior to this patch - just re-add all existing devices when there is a problem.\n\nA device also makes it into seen_devices on L962. This device might have been partially added (like iptables only) in the previous iteration that gave an exception later on. This device will not be added to registered_devices.\n\nSo I think the complexity is because scan_devices is now doing much more than it was originally intended. And now it is necessary to maintain another tempory variables in the main loop and pass it so it continues to work in various senarios.\n\nI have started on a new patchset that eliminates these (seen_devices, registered_devices, updated_devices_copy) and just passes the previous device_info to scan_devices. I think it simplfies things, but the patch has many more lines of code and the refactor probably goes beyond the scope of this bug. Anyway I\u0027ll push it up just to get feedback.","commit_id":"7e42aec19b1bf4a14849deb4c96949051ee723ae"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"f9509e59c633a07066c7ec13da69b124e5ff215e","unresolved":false,"context_lines":[{"line_number":933,"context_line":"        curr_devices \u003d self.br_mgr.get_tap_devices()"},{"line_number":934,"context_line":"        device_info \u003d {}"},{"line_number":935,"context_line":"        device_info[\u0027current\u0027] \u003d curr_devices"},{"line_number":936,"context_line":"        device_info[\u0027added\u0027] \u003d curr_devices - registered_devices"},{"line_number":937,"context_line":"        # we don\u0027t want to process updates for devices that don\u0027t exist"},{"line_number":938,"context_line":"        device_info[\u0027updated\u0027] \u003d updated_devices \u0026 curr_devices"},{"line_number":939,"context_line":"        # we need to clean up after devices are removed"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAXH%2F%2Fu2U%3D","line":936,"in_reply_to":"AAAAXH%2F%2Fz1A%3D","updated":"2014-04-17 20:50:12.000000000","message":"Darragh, I think you understood the question correctly.  I was suggesting using seen_devices in place of registered_devices here as well as on line 940.  My thinking was that this might remove the need for registered_devices.  However, I did not consider why registered_devices was being cleared on L963.  I still probably don\u0027t fully understand that.\n\nA device makes it in to the seen_devices set only when it was already successfully added and processed by process_network_devices.  My thinking was that it would not hurt to subtract these devices from the added set on L936.  But, maybe I\u0027ve taken an overly simplistic view of the logic here.\n\nI think you\u0027re right, this is overly complex.  Any work that could reduce the complexity of this logic would help.  :)","commit_id":"7e42aec19b1bf4a14849deb4c96949051ee723ae"},{"author":{"_account_id":6598,"name":"Berezovsky Irena","email":"irenab.dev@gmail.com","username":"irenab"},"change_message_id":"033510a6de6826ebe731a3fb23860bdf88681ebd","unresolved":false,"context_lines":[{"line_number":965,"context_line":"            # Save updated devices dict to perform rollback in case"},{"line_number":966,"context_line":"            # resync would be needed, and then clear self.updated_devices."},{"line_number":967,"context_line":"            # As the greenthread should not yield between these"},{"line_number":968,"context_line":"            # two statements, this will should be thread-safe."},{"line_number":969,"context_line":"            updated_devices_copy \u003d self.updated_devices"},{"line_number":970,"context_line":"            self.updated_devices \u003d set()"},{"line_number":971,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAW3%2F%2F1Gs%3D","line":968,"updated":"2014-04-10 05:56:58.000000000","message":"nit: please drop \u0027will\u0027, and leave \u0027this should be thread-safe\u0027","commit_id":"7e42aec19b1bf4a14849deb4c96949051ee723ae"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"f9509e59c633a07066c7ec13da69b124e5ff215e","unresolved":false,"context_lines":[{"line_number":965,"context_line":"            # Save updated devices dict to perform rollback in case"},{"line_number":966,"context_line":"            # resync would be needed, and then clear self.updated_devices."},{"line_number":967,"context_line":"            # As the greenthread should not yield between these"},{"line_number":968,"context_line":"            # two statements, this will should be thread-safe."},{"line_number":969,"context_line":"            updated_devices_copy \u003d self.updated_devices"},{"line_number":970,"context_line":"            self.updated_devices \u003d set()"},{"line_number":971,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAXH%2F%2Fu%2FQ%3D","line":968,"in_reply_to":"AAAAW3%2F%2F1Gs%3D","updated":"2014-04-17 20:50:12.000000000","message":"That seems to be out of the scope of this patch to me.","commit_id":"7e42aec19b1bf4a14849deb4c96949051ee723ae"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"06068f84c52ac7512e4d0f28e3da68eacf8f18d1","unresolved":false,"context_lines":[{"line_number":1012,"context_line":"    LOG.info(_(\"Agent initialized successfully, now running... \"))"},{"line_number":1013,"context_line":"    agent.daemon_loop()"},{"line_number":1014,"context_line":"    sys.exit(0)"},{"line_number":1015,"context_line":""},{"line_number":1016,"context_line":""},{"line_number":1017,"context_line":"if __name__ \u003d\u003d \"__main__\":"},{"line_number":1018,"context_line":"    main()"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAXH%2F%2FbE0%3D","side":"PARENT","line":1015,"updated":"2014-04-21 22:33:05.000000000","message":"Don\u0027t remove this line.","commit_id":"ed1041c50192b4dbe4a8127f065a7c381d79ae97"},{"author":{"_account_id":2592,"name":"Mark McClain","email":"mark@mcclain.xyz","username":"markmcclain"},"change_message_id":"0f530a87d63e30d7ca98baff795fe05ef584dfe9","unresolved":false,"context_lines":[{"line_number":970,"context_line":""},{"line_number":971,"context_line":"    def daemon_loop(self):"},{"line_number":972,"context_line":"        LOG.info(_(\"LinuxBridge Agent RPC Daemon Started!\"))"},{"line_number":973,"context_line":"        device_info \u003d {\u0027current\u0027: set(),"},{"line_number":974,"context_line":"                       \u0027added\u0027: set(),"},{"line_number":975,"context_line":"                       \u0027updated\u0027: set(),"},{"line_number":976,"context_line":"                       \u0027removed\u0027: set()}"}],"source_content_type":"text/x-python","patch_set":5,"id":"1ae5cdf2_2ac72365","line":973,"updated":"2014-06-05 17:49:14.000000000","message":"seems like a small class should be written instead of constructing a dict","commit_id":"2198f8240fcbe1db49abf11c78e3f81855571e0e"},{"author":{"_account_id":2733,"name":"Darragh O\u0027Reilly","email":"doreilly@suse.com","username":"darragh-oreilly"},"change_message_id":"bb66e0196f41e9511e8a9fda69b78d446bd7e516","unresolved":false,"context_lines":[{"line_number":970,"context_line":""},{"line_number":971,"context_line":"    def daemon_loop(self):"},{"line_number":972,"context_line":"        LOG.info(_(\"LinuxBridge Agent RPC Daemon Started!\"))"},{"line_number":973,"context_line":"        device_info \u003d {\u0027current\u0027: set(),"},{"line_number":974,"context_line":"                       \u0027added\u0027: set(),"},{"line_number":975,"context_line":"                       \u0027updated\u0027: set(),"},{"line_number":976,"context_line":"                       \u0027removed\u0027: set()}"}],"source_content_type":"text/x-python","patch_set":5,"id":"dab895d6_274bb85a","line":973,"in_reply_to":"1ae5cdf2_2ac72365","updated":"2014-06-30 06:38:45.000000000","message":"I\u0027m not sure what you mean. device_info gets passed to other methods and they expect a dict. I moved the initialisation into scan_devices() - closer to where it is first used - I think it\u0027s a bit better.","commit_id":"2198f8240fcbe1db49abf11c78e3f81855571e0e"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"bf6117ef3ce164512ec46809803ba260bac60b42","unresolved":false,"context_lines":[{"line_number":970,"context_line":""},{"line_number":971,"context_line":"    def daemon_loop(self):"},{"line_number":972,"context_line":"        LOG.info(_(\"LinuxBridge Agent RPC Daemon Started!\"))"},{"line_number":973,"context_line":"        device_info \u003d {\u0027current\u0027: set(),"},{"line_number":974,"context_line":"                       \u0027added\u0027: set(),"},{"line_number":975,"context_line":"                       \u0027updated\u0027: set(),"},{"line_number":976,"context_line":"                       \u0027removed\u0027: set()}"}],"source_content_type":"text/x-python","patch_set":5,"id":"baada198_bfa42958","line":973,"in_reply_to":"dab895d6_274bb85a","updated":"2014-07-01 03:41:23.000000000","message":"I agree that a small class would be an improvement over a dict here.  However, I don\u0027t think the changes to make that happen fit within the scope of this patch set.","commit_id":"2198f8240fcbe1db49abf11c78e3f81855571e0e"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"0d02c7a28691076d320e5efcd75c08fc381f42ce","unresolved":false,"context_lines":[{"line_number":948,"context_line":""},{"line_number":949,"context_line":"        if previous is None:"},{"line_number":950,"context_line":"            # This is the first iteration of daemon_loop()."},{"line_number":951,"context_line":"            previous \u003d {\u0027added\u0027: set(),"},{"line_number":952,"context_line":"                        \u0027current\u0027: set(),"},{"line_number":953,"context_line":"                        \u0027updated\u0027: set(),"},{"line_number":954,"context_line":"                        \u0027removed\u0027: set()}"}],"source_content_type":"text/x-python","patch_set":6,"id":"baada198_ff2141b5","line":951,"updated":"2014-07-01 03:48:43.000000000","message":"nit:  I liked PS5 better.  I don\u0027t understand the motivation for changing it this way.","commit_id":"6bf67e0a576bd573042742f9b4bfc464d544c837"}],"neutron/tests/unit/linuxbridge/test_lb_neutron_agent.py":[{"author":{"_account_id":2733,"name":"Darragh O\u0027Reilly","email":"doreilly@suse.com","username":"darragh-oreilly"},"change_message_id":"a02b5b13fe83b2a637f698b054daf05519380f29","unresolved":false,"context_lines":[{"line_number":114,"context_line":"                                                                          0,"},{"line_number":115,"context_line":"                                                                          None)"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"    def mock_scan_devices(self, expected, mock_current,"},{"line_number":118,"context_line":"                          registered_devices, seen_devices, updated_devices):"},{"line_number":119,"context_line":"        self.agent.br_mgr \u003d mock.Mock()"},{"line_number":120,"context_line":"        self.agent.br_mgr.get_tap_devices.return_value \u003d mock_current"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAXH%2F%2Fpq8%3D","line":117,"updated":"2014-04-18 14:14:48.000000000","message":"forgot to remove this.","commit_id":"8b4283f8a0423ad485b0cbdcff9456cfebebe963"}]}