)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":30247,"name":"Ilya Maximets","email":"i.maximets@ovn.org","username":"i.maximets"},"change_message_id":"83b24121a087b7868a413644bb6c3133dfd2cd10","unresolved":false,"context_lines":[{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Change-Id: I3ae4cbde638fe12dc925d5489b5e002e573c0a83"},{"line_number":18,"context_line":"Resolves-Bug: 1825916"},{"line_number":19,"context_line":"Signed-off-by: Yash Gupta \u003cy.gupta@samsung.com\u003e"},{"line_number":20,"context_line":"Signed-off-by: Yash Gupta \u003cygupta@samsung.com\u003e"},{"line_number":21,"context_line":"Signed-off-by: Yash Gupta \u003cy.gupta@samsung.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"bfb3d3c7_5bb30799","line":21,"range":{"start_line":19,"start_character":0,"end_line":21,"end_character":47},"updated":"2019-05-24 08:08:34.000000000","message":"Why so many Sign-off\u0027s? And there is a typo in one of them, I guess.","commit_id":"62970b1f5b2b4d0bc46780306460b22a7aa853ed"}],"kuryr_kubernetes/cni/binding/sriov.py":[{"author":{"_account_id":30247,"name":"Ilya Maximets","email":"i.maximets@ovn.org","username":"i.maximets"},"change_message_id":"50e9cb306df2efd8e15b99548a780f8fb3f78b1a","unresolved":false,"context_lines":[{"line_number":30,"context_line":"CONF \u003d cfg.CONF"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"def release_lock_object(func):"},{"line_number":34,"context_line":"    def wrapped(self, *args, **kwargs):"},{"line_number":35,"context_line":"        try:"},{"line_number":36,"context_line":"            return func(self, *args, **kwargs)"}],"source_content_type":"text/x-python","patch_set":2,"id":"dfbec78f_473e212a","line":33,"range":{"start_line":33,"start_character":0,"end_line":33,"end_character":30},"updated":"2019-05-06 07:10:20.000000000","message":"Why you\u0027re moving this function out of the class?","commit_id":"f078be8578230befaa9e4084239d5f31a75f4ff8"},{"author":{"_account_id":30247,"name":"Ilya Maximets","email":"i.maximets@ovn.org","username":"i.maximets"},"change_message_id":"23714a00367edc9e8002029e17d00a7280d2fcb4","unresolved":false,"context_lines":[{"line_number":30,"context_line":"CONF \u003d cfg.CONF"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"def release_lock_object(func):"},{"line_number":34,"context_line":"    def wrapped(self, *args, **kwargs):"},{"line_number":35,"context_line":"        try:"},{"line_number":36,"context_line":"            return func(self, *args, **kwargs)"}],"source_content_type":"text/x-python","patch_set":2,"id":"dfbec78f_b0ab87d4","line":33,"range":{"start_line":33,"start_character":0,"end_line":33,"end_character":30},"in_reply_to":"dfbec78f_1fbce4da","updated":"2019-05-07 08:08:13.000000000","message":"There is only one class in this module. To reuse it in other modules, this decorator should be moved to some more generic module. This will require one more move.\n\nThe most important here is that moving the entire function hides the real changes you made in this function from the git point of view. It\u0027s hard to track changes this way. If someone will need it, this function moving could be done along with the change that will use it out of this class. This will keep the git history more or less consistent.\n\nAlso, decorator has \u0027self\u0027 argument that forces it using inside the class. So, defining it out of the class is strange.","commit_id":"f078be8578230befaa9e4084239d5f31a75f4ff8"},{"author":{"_account_id":28082,"name":"Yash Gupta","email":"y.gupta@samsung.com","username":"y.gupta"},"change_message_id":"b2e5a467a7e20cbd4de43dc3bd44021c62e6bdaf","unresolved":false,"context_lines":[{"line_number":30,"context_line":"CONF \u003d cfg.CONF"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"def release_lock_object(func):"},{"line_number":34,"context_line":"    def wrapped(self, *args, **kwargs):"},{"line_number":35,"context_line":"        try:"},{"line_number":36,"context_line":"            return func(self, *args, **kwargs)"}],"source_content_type":"text/x-python","patch_set":2,"id":"dfbec78f_1fbce4da","line":33,"range":{"start_line":33,"start_character":0,"end_line":33,"end_character":30},"in_reply_to":"dfbec78f_473e212a","updated":"2019-05-07 02:00:24.000000000","message":"Maybe we can reuse this decorator for other drivers that need a lock to prevent race conditions in the future.\n\nIf it is recommended to have it inside the class, I would appreciate to know the reasoning","commit_id":"f078be8578230befaa9e4084239d5f31a75f4ff8"},{"author":{"_account_id":28082,"name":"Yash Gupta","email":"y.gupta@samsung.com","username":"y.gupta"},"change_message_id":"dd014bc4ba7df7db6354838d1bcfe6c66a010f06","unresolved":false,"context_lines":[{"line_number":30,"context_line":"CONF \u003d cfg.CONF"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"def release_lock_object(func):"},{"line_number":34,"context_line":"    def wrapped(self, *args, **kwargs):"},{"line_number":35,"context_line":"        try:"},{"line_number":36,"context_line":"            return func(self, *args, **kwargs)"}],"source_content_type":"text/x-python","patch_set":2,"id":"dfbec78f_aa28b933","line":33,"range":{"start_line":33,"start_character":0,"end_line":33,"end_character":30},"in_reply_to":"dfbec78f_b0ab87d4","updated":"2019-05-10 02:45:18.000000000","message":"While I agree with your first two points, decorator itself doesn\u0027t use \u0027self\u0027... it is used by the wrapper function (so it is valid to define the decorator outside the class)\n\nFurther, I think as the decorator doesn\u0027t have \u0027self\u0027 as the first argument, it should be a staticmethod to be part of the class.. but the decorator need to be a callable, and using @staticmethod on the decorator makes it otherwise. So we are left with a class member which is not a staticmethod, but still doesn\u0027t have self as the first argument.  I think this is not a good practice in python.\n\nHowever, I have made this decorator part of the class again, as moving it outside the class hides the real changes.","commit_id":"f078be8578230befaa9e4084239d5f31a75f4ff8"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"cb43e17f6ef50a35bb5a0a1c23390d6b27a61312","unresolved":false,"context_lines":[{"line_number":51,"context_line":"    def connect(self, vif, ifname, netns, container_id):"},{"line_number":52,"context_line":"        physnet \u003d vif.physnet"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"        h_ipdb \u003d b_base.get_ipdb()"},{"line_number":55,"context_line":"        c_ipdb \u003d b_base.get_ipdb(netns)"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"        pf_names \u003d self._get_host_pf_names(physnet)"},{"line_number":58,"context_line":"        vf_name, vf_index, pf, pci_info \u003d self._get_available_vf_info(pf_names)"}],"source_content_type":"text/x-python","patch_set":5,"id":"bfb3d3c7_8931640e","side":"PARENT","line":55,"range":{"start_line":54,"start_character":0,"end_line":55,"end_character":39},"updated":"2019-05-22 13:55:51.000000000","message":"Oh crap, why didn\u0027t I noticed this when reviewing!","commit_id":"df31c6075aea15db04eababaf65d3fefd0bdfbc1"}],"kuryr_kubernetes/tests/unit/cni/test_binding.py":[{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"a0201e854a0daea9759f910077b0874f93bd9eee","unresolved":false,"context_lines":[{"line_number":222,"context_line":"                \u0027_get_available_vf_info\u0027)"},{"line_number":223,"context_line":"    @mock.patch(\u0027kuryr_kubernetes.cni.binding.sriov.VIFSriovDriver.\u0027"},{"line_number":224,"context_line":"                \u0027_set_vf_mac\u0027)"},{"line_number":225,"context_line":"    @mock.patch(\u0027kuryr_kubernetes.cni.binding.sriov.VIFSriovDriver.\u0027"},{"line_number":226,"context_line":"                \u0027_save_pci_info\u0027)"},{"line_number":227,"context_line":"    def test_connect(self, m_save_pci_info, m_set_vf_mac, m_avail_vf_info,"},{"line_number":228,"context_line":"                     m_host_pf_names):"},{"line_number":229,"context_line":"        m_avail_vf_info.return_value \u003d [self.ifname, 1,"}],"source_content_type":"text/x-python","patch_set":6,"id":"bfb3d3c7_4c784829","line":226,"range":{"start_line":225,"start_character":0,"end_line":226,"end_character":33},"updated":"2019-05-23 08:43:10.000000000","message":"Interestingly I don\u0027t know why this wasn\u0027t triggered by the patch that added that method. Anyway it\u0027s great you\u0027ve found the culprit. :)","commit_id":"62970b1f5b2b4d0bc46780306460b22a7aa853ed"}]}