)]}'
{"smaug/services/protection/protectable_registry.py":[{"author":{"_account_id":18266,"name":"zengchen","email":"chenzeng2@huawei.com","username":"zengchen"},"change_message_id":"472682894ccf81fdc08328f7d5a8e8c0693cf936","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        \"\"\"Load all protectable plugins configured and register them."},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"        \"\"\""},{"line_number":40,"context_line":"        mgr \u003d extension.ExtensionManager("},{"line_number":41,"context_line":"            namespace\u003d\u0027smaug.protectables\u0027,"},{"line_number":42,"context_line":"            invoke_on_load\u003dTrue,"},{"line_number":43,"context_line":"            on_load_failure_callback\u003d_raise_extension_exception)"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a122d0e_3e4bdbb2","line":40,"range":{"start_line":40,"start_character":24,"end_line":40,"end_character":40},"updated":"2016-05-05 07:52:30.000000000","message":"According to the design of Stevedore, it is better to use stevedore.driver.DriverManager instead, because the map between the name of plugin and its realization is 1:1","commit_id":"2c384d0aa380baf3f795366a29380696351a9356"},{"author":{"_account_id":18266,"name":"zengchen","email":"chenzeng2@huawei.com","username":"zengchen"},"change_message_id":"472682894ccf81fdc08328f7d5a8e8c0693cf936","unresolved":false,"context_lines":[{"line_number":39,"context_line":"        \"\"\""},{"line_number":40,"context_line":"        mgr \u003d extension.ExtensionManager("},{"line_number":41,"context_line":"            namespace\u003d\u0027smaug.protectables\u0027,"},{"line_number":42,"context_line":"            invoke_on_load\u003dTrue,"},{"line_number":43,"context_line":"            on_load_failure_callback\u003d_raise_extension_exception)"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"        for e in mgr:"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a122d0e_be9b6b16","line":42,"range":{"start_line":42,"start_character":12,"end_line":42,"end_character":26},"updated":"2016-05-05 07:52:30.000000000","message":"I think it is better to set invoke_on_load\u003dFalse to just load the classes. The member functions of ProtectablePlugin should not be all the method of instance, some functions like get_parent_resource_types should be the method of class.","commit_id":"2c384d0aa380baf3f795366a29380696351a9356"},{"author":{"_account_id":20883,"name":"Yuval Brik","email":"yuval@brik.org.il","username":"yuvalbr"},"change_message_id":"767c01b2400b4f67aa1007e13f15bda6d079942c","unresolved":false,"context_lines":[{"line_number":39,"context_line":"        \"\"\""},{"line_number":40,"context_line":"        mgr \u003d extension.ExtensionManager("},{"line_number":41,"context_line":"            namespace\u003d\u0027smaug.protectables\u0027,"},{"line_number":42,"context_line":"            invoke_on_load\u003dTrue,"},{"line_number":43,"context_line":"            on_load_failure_callback\u003d_raise_extension_exception)"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"        for e in mgr:"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a122d0e_27ce265a","line":42,"range":{"start_line":42,"start_character":12,"end_line":42,"end_character":26},"in_reply_to":"1a122d0e_be9b6b16","updated":"2016-05-05 13:42:43.000000000","message":"Agree","commit_id":"2c384d0aa380baf3f795366a29380696351a9356"},{"author":{"_account_id":18266,"name":"zengchen","email":"chenzeng2@huawei.com","username":"zengchen"},"change_message_id":"472682894ccf81fdc08328f7d5a8e8c0693cf936","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        self._plugin_map[plugin.get_resource_type()] \u003d plugin"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"    def _get_protectable(self, context, resource_type):"},{"line_number":52,"context_line":"        protectable \u003d self._plugin_map[resource_type].instance(context)"},{"line_number":53,"context_line":"        return protectable"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def list_resource_types(self):"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a122d0e_9eb78f84","line":52,"range":{"start_line":52,"start_character":54,"end_line":52,"end_character":62},"updated":"2016-05-05 07:52:30.000000000","message":"It is better to create instance by self._plugin_map[resource_type](context) for simple if self._plugin_map[resource_type] is the class.","commit_id":"2c384d0aa380baf3f795366a29380696351a9356"},{"author":{"_account_id":20883,"name":"Yuval Brik","email":"yuval@brik.org.il","username":"yuvalbr"},"change_message_id":"767c01b2400b4f67aa1007e13f15bda6d079942c","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        self._plugin_map[plugin.get_resource_type()] \u003d plugin"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"    def _get_protectable(self, context, resource_type):"},{"line_number":52,"context_line":"        protectable \u003d self._plugin_map[resource_type].instance(context)"},{"line_number":53,"context_line":"        return protectable"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def list_resource_types(self):"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a122d0e_e7e70ee1","line":52,"range":{"start_line":52,"start_character":54,"end_line":52,"end_character":62},"in_reply_to":"1a122d0e_9eb78f84","updated":"2016-05-05 13:42:43.000000000","message":"When invoke_on_load is False we should create an instance as you said.","commit_id":"2c384d0aa380baf3f795366a29380696351a9356"},{"author":{"_account_id":18266,"name":"zengchen","email":"chenzeng2@huawei.com","username":"zengchen"},"change_message_id":"472682894ccf81fdc08328f7d5a8e8c0693cf936","unresolved":false,"context_lines":[{"line_number":57,"context_line":""},{"line_number":58,"context_line":"        :return: The list of supported resource types."},{"line_number":59,"context_line":"        \"\"\""},{"line_number":60,"context_line":"        return [type for type in six.iterkeys(self._plugin_map)]"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    def get_protectable_resource_plugin(self, resource_type):"},{"line_number":63,"context_line":"        \"\"\"Get the protectable plugin with the specified type.\"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"1a122d0e_fe0cc343","line":60,"range":{"start_line":60,"start_character":37,"end_line":60,"end_character":45},"updated":"2016-05-05 07:52:30.000000000","message":"I think it is simpler to return the types by \u0027return self._plugin_map.keys()\u0027","commit_id":"2c384d0aa380baf3f795366a29380696351a9356"},{"author":{"_account_id":20883,"name":"Yuval Brik","email":"yuval@brik.org.il","username":"yuvalbr"},"change_message_id":"767c01b2400b4f67aa1007e13f15bda6d079942c","unresolved":false,"context_lines":[{"line_number":57,"context_line":""},{"line_number":58,"context_line":"        :return: The list of supported resource types."},{"line_number":59,"context_line":"        \"\"\""},{"line_number":60,"context_line":"        return [type for type in six.iterkeys(self._plugin_map)]"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    def get_protectable_resource_plugin(self, resource_type):"},{"line_number":63,"context_line":"        \"\"\"Get the protectable plugin with the specified type.\"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"1a122d0e_3e1476e7","line":60,"range":{"start_line":60,"start_character":37,"end_line":60,"end_character":45},"in_reply_to":"1a122d0e_fe0cc343","updated":"2016-05-05 13:42:43.000000000","message":"`.keys()` is different in python 2 and 3 (returning a dict_keys object in python 3)\nWe should use `list(self._plugin_map)` to get the desired effect","commit_id":"2c384d0aa380baf3f795366a29380696351a9356"},{"author":{"_account_id":18266,"name":"zengchen","email":"chenzeng2@huawei.com","username":"zengchen"},"change_message_id":"472682894ccf81fdc08328f7d5a8e8c0693cf936","unresolved":false,"context_lines":[{"line_number":102,"context_line":"        def fetch_dependent_resources_context(resource):"},{"line_number":103,"context_line":"            return self.fetch_dependent_resources(context, resource)"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"        return build_graph("},{"line_number":106,"context_line":"            start_nodes\u003dresources,"},{"line_number":107,"context_line":"            get_child_nodes_func\u003dfetch_dependent_resources_context,"},{"line_number":108,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a122d0e_e9dd2f71","line":105,"range":{"start_line":105,"start_character":15,"end_line":105,"end_character":26},"updated":"2016-05-05 07:52:30.000000000","message":"Maybe it is better to change another name for \u0027build_graph\u0027, because it is same with the name of member function. It is easier to be confused.","commit_id":"2c384d0aa380baf3f795366a29380696351a9356"}],"smaug/services/protection/protection_plugin.py":[{"author":{"_account_id":19346,"name":"yinwei","email":"yinweiishere@icloud.com","username":"yinwei"},"change_message_id":"a8af34e6f8b864f05396a8bc30f4f14c4361d8c5","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        pass"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    @abc.abstractmethod"},{"line_number":46,"context_line":"    def get_restore_schema(self, resource_type):"},{"line_number":47,"context_line":"        # TODO(wangliuan)"},{"line_number":48,"context_line":"        pass"},{"line_number":49,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"dab17558_9006521a","line":46,"updated":"2016-05-16 03:12:01.000000000","message":"the schema is defined per protection plugin instance,while resource type is fixed when protection plugin get instance initialized.   Why resource_type is here?","commit_id":"599dbf9a883ba7a62094fe82bea02d86a2e0ea07"},{"author":{"_account_id":20883,"name":"Yuval Brik","email":"yuval@brik.org.il","username":"yuvalbr"},"change_message_id":"db66b7a9534a7df49c38d40b69012322c041116e","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        pass"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    @abc.abstractmethod"},{"line_number":46,"context_line":"    def get_restore_schema(self, resource_type):"},{"line_number":47,"context_line":"        # TODO(wangliuan)"},{"line_number":48,"context_line":"        pass"},{"line_number":49,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"dab17558_c8974f38","line":46,"in_reply_to":"dab17558_1cdfe1d6","updated":"2016-05-17 05:46:31.000000000","message":"get_supported_resources_types() can return more than 1 resource type.\nIn that case, the protection plugin will be used for these resources. Therefor, when the I/S asks for the schema, it must specify for which resource.","commit_id":"599dbf9a883ba7a62094fe82bea02d86a2e0ea07"},{"author":{"_account_id":19346,"name":"yinwei","email":"yinweiishere@icloud.com","username":"yinwei"},"change_message_id":"62d8bf6ae0e14d9ebd9a08d3543f26ba38e4774f","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        pass"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    @abc.abstractmethod"},{"line_number":46,"context_line":"    def get_restore_schema(self, resource_type):"},{"line_number":47,"context_line":"        # TODO(wangliuan)"},{"line_number":48,"context_line":"        pass"},{"line_number":49,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"dab17558_1cdfe1d6","line":46,"in_reply_to":"dab17558_2a8e0e79","updated":"2016-05-17 03:04:20.000000000","message":"could you pls. give an example","commit_id":"599dbf9a883ba7a62094fe82bea02d86a2e0ea07"},{"author":{"_account_id":13070,"name":"Eran Gampel","email":"eran@gampel.net","username":"Gampel"},"change_message_id":"c1a36b1e80ccb6d8698dc881cce8473f9c1c4c7b","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        pass"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    @abc.abstractmethod"},{"line_number":46,"context_line":"    def get_restore_schema(self, resource_type):"},{"line_number":47,"context_line":"        # TODO(wangliuan)"},{"line_number":48,"context_line":"        pass"},{"line_number":49,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"dab17558_82d88354","line":46,"in_reply_to":"dab17558_621c7f6a","updated":"2016-05-17 07:12:30.000000000","message":"The requirement that we got from few vendors is that they would like to have for example  a protection plugin that protect VM and Volume. There back-end does that this way today. The solution that we discussed in the past was to allow a protection plugin to protect more then one resource type, but our workflow will call it in DFS for each of the resources.","commit_id":"599dbf9a883ba7a62094fe82bea02d86a2e0ea07"},{"author":{"_account_id":18266,"name":"zengchen","email":"chenzeng2@huawei.com","username":"zengchen"},"change_message_id":"5efe502a2dc30a1a92412bcce379897da59996b9","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        pass"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    @abc.abstractmethod"},{"line_number":46,"context_line":"    def get_restore_schema(self, resource_type):"},{"line_number":47,"context_line":"        # TODO(wangliuan)"},{"line_number":48,"context_line":"        pass"},{"line_number":49,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"dab17558_3800e801","line":46,"in_reply_to":"dab17558_82d88354","updated":"2016-05-17 08:19:19.000000000","message":"got it. thanks, Eran.","commit_id":"599dbf9a883ba7a62094fe82bea02d86a2e0ea07"},{"author":{"_account_id":20883,"name":"Yuval Brik","email":"yuval@brik.org.il","username":"yuvalbr"},"change_message_id":"46b307c48eafa234ce6c79268176bc0aac447354","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        pass"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    @abc.abstractmethod"},{"line_number":46,"context_line":"    def get_restore_schema(self, resource_type):"},{"line_number":47,"context_line":"        # TODO(wangliuan)"},{"line_number":48,"context_line":"        pass"},{"line_number":49,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"dab17558_2a8e0e79","line":46,"in_reply_to":"dab17558_9006521a","updated":"2016-05-16 05:54:58.000000000","message":"Because a protection plugin can support more than 1 resource type.","commit_id":"599dbf9a883ba7a62094fe82bea02d86a2e0ea07"},{"author":{"_account_id":18266,"name":"zengchen","email":"chenzeng2@huawei.com","username":"zengchen"},"change_message_id":"10c4ab5de7bc96a91abf74a60f512df9784fd9a0","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        pass"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    @abc.abstractmethod"},{"line_number":46,"context_line":"    def get_restore_schema(self, resource_type):"},{"line_number":47,"context_line":"        # TODO(wangliuan)"},{"line_number":48,"context_line":"        pass"},{"line_number":49,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"dab17558_621c7f6a","line":46,"in_reply_to":"dab17558_c8974f38","updated":"2016-05-17 07:04:42.000000000","message":"Sorry, why does a plugin support more than 1 resource type? If it can, the schema should be the same for different resources, otherwise, it is better for one resource per plugin.","commit_id":"599dbf9a883ba7a62094fe82bea02d86a2e0ea07"}],"smaug/services/protection/provider.py":[{"author":{"_account_id":18266,"name":"zengchen","email":"chenzeng2@huawei.com","username":"zengchen"},"change_message_id":"472682894ccf81fdc08328f7d5a8e8c0693cf936","unresolved":false,"context_lines":[{"line_number":62,"context_line":"        self.checkpoint_collection \u003d None"},{"line_number":63,"context_line":"        self._bank_plugin \u003d None"},{"line_number":64,"context_line":"        self._plugin_map \u003d {}"},{"line_number":65,"context_line":"        self.protectable_registry \u003d ProtectableRegistry()"},{"line_number":66,"context_line":"        self.protectable_registry.load_plugins()"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"        if hasattr(self._config.provider, \u0027bank\u0027) \\"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a122d0e_e441d63a","line":65,"range":{"start_line":65,"start_character":36,"end_line":65,"end_character":55},"updated":"2016-05-05 07:52:30.000000000","message":"If it need to create an instance of ProtectableRegistry here, it is better to make ProtectableRegistry be a singleton. Because the protection/manager.py has created an instance of it.\n\nBTW, what is self.protectable_registry used for?","commit_id":"2c384d0aa380baf3f795366a29380696351a9356"},{"author":{"_account_id":20883,"name":"Yuval Brik","email":"yuval@brik.org.il","username":"yuvalbr"},"change_message_id":"767c01b2400b4f67aa1007e13f15bda6d079942c","unresolved":false,"context_lines":[{"line_number":62,"context_line":"        self.checkpoint_collection \u003d None"},{"line_number":63,"context_line":"        self._bank_plugin \u003d None"},{"line_number":64,"context_line":"        self._plugin_map \u003d {}"},{"line_number":65,"context_line":"        self.protectable_registry \u003d ProtectableRegistry()"},{"line_number":66,"context_line":"        self.protectable_registry.load_plugins()"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"        if hasattr(self._config.provider, \u0027bank\u0027) \\"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a122d0e_47a3629e","line":65,"range":{"start_line":65,"start_character":36,"end_line":65,"end_character":55},"in_reply_to":"1a122d0e_e441d63a","updated":"2016-05-05 13:42:43.000000000","message":"ProtectableRegistry is borg-like. You can create many of it, but they have a shared state: _plugin_map.\nIt is used here to load the plugins.","commit_id":"2c384d0aa380baf3f795366a29380696351a9356"},{"author":{"_account_id":18266,"name":"zengchen","email":"chenzeng2@huawei.com","username":"zengchen"},"change_message_id":"faf686fbd93f3ac4fc01cdad956ecd6780058834","unresolved":false,"context_lines":[{"line_number":63,"context_line":"        self._bank_plugin \u003d None"},{"line_number":64,"context_line":"        self._plugin_map \u003d {}"},{"line_number":65,"context_line":"        self.protectable_registry \u003d ProtectableRegistry()"},{"line_number":66,"context_line":"        self.protectable_registry.load_plugins()"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"        if hasattr(self._config.provider, \u0027bank\u0027) \\"},{"line_number":69,"context_line":"                and not self._config.provider.bank:"}],"source_content_type":"text/x-python","patch_set":4,"id":"dab17558_13266768","line":66,"range":{"start_line":66,"start_character":34,"end_line":66,"end_character":46},"updated":"2016-05-06 02:54:48.000000000","message":"You use a shared _plugin_map to reach the same result as singleton. I prefer the singleton. Because all the plugins have been loaded after load_plugin is invoked in the manager.py, other modules just use the functions of ProtectableRegistry and no need to load plugins again.","commit_id":"faed7d12de49740dfe7f6f851609b29d3f42f46b"},{"author":{"_account_id":18266,"name":"zengchen","email":"chenzeng2@huawei.com","username":"zengchen"},"change_message_id":"faf686fbd93f3ac4fc01cdad956ecd6780058834","unresolved":false,"context_lines":[{"line_number":63,"context_line":"        self._bank_plugin \u003d None"},{"line_number":64,"context_line":"        self._plugin_map \u003d {}"},{"line_number":65,"context_line":"        self.protectable_registry \u003d ProtectableRegistry()"},{"line_number":66,"context_line":"        self.protectable_registry.load_plugins()"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"        if hasattr(self._config.provider, \u0027bank\u0027) \\"},{"line_number":69,"context_line":"                and not self._config.provider.bank:"}],"source_content_type":"text/x-python","patch_set":4,"id":"dab17558_53dd8f6f","line":66,"range":{"start_line":66,"start_character":34,"end_line":66,"end_character":46},"updated":"2016-05-06 02:54:48.000000000","message":"load_plugins is invoked several times. I still","commit_id":"faed7d12de49740dfe7f6f851609b29d3f42f46b"},{"author":{"_account_id":18266,"name":"zengchen","email":"chenzeng2@huawei.com","username":"zengchen"},"change_message_id":"131444fb47911dc4fbe14f601df66a8f4b3eaf14","unresolved":false,"context_lines":[{"line_number":63,"context_line":"        self._bank_plugin \u003d None"},{"line_number":64,"context_line":"        self._plugin_map \u003d {}"},{"line_number":65,"context_line":"        self.protectable_registry \u003d ProtectableRegistry()"},{"line_number":66,"context_line":"        self.protectable_registry.load_plugins()"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"        if hasattr(self._config.provider, \u0027bank\u0027) \\"},{"line_number":69,"context_line":"                and not self._config.provider.bank:"}],"source_content_type":"text/x-python","patch_set":4,"id":"dab17558_1335c745","line":66,"range":{"start_line":66,"start_character":34,"end_line":66,"end_character":46},"updated":"2016-05-06 03:06:01.000000000","message":"please,just look at the first comment. sorry.","commit_id":"faed7d12de49740dfe7f6f851609b29d3f42f46b"},{"author":{"_account_id":18266,"name":"zengchen","email":"chenzeng2@huawei.com","username":"zengchen"},"change_message_id":"ba046f643ce69e4762d730a89c9cc325c7b2edfb","unresolved":false,"context_lines":[{"line_number":63,"context_line":"        self._bank_plugin \u003d None"},{"line_number":64,"context_line":"        self._plugin_map \u003d {}"},{"line_number":65,"context_line":"        self.protectable_registry \u003d ProtectableRegistry()"},{"line_number":66,"context_line":"        self.protectable_registry.load_plugins()"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"        if hasattr(self._config.provider, \u0027bank\u0027) \\"},{"line_number":69,"context_line":"                and not self._config.provider.bank:"}],"source_content_type":"text/x-python","patch_set":4,"id":"dab17558_13ddc742","line":66,"range":{"start_line":66,"start_character":34,"end_line":66,"end_character":46},"in_reply_to":"dab17558_53dd8f6f","updated":"2016-05-06 02:55:42.000000000","message":"Done","commit_id":"faed7d12de49740dfe7f6f851609b29d3f42f46b"},{"author":{"_account_id":18266,"name":"zengchen","email":"chenzeng2@huawei.com","username":"zengchen"},"change_message_id":"b30a2f7c43efeedd3ce4da9104fd5679cea2f1f0","unresolved":false,"context_lines":[{"line_number":65,"context_line":"        self.protectable_registry \u003d ProtectableRegistry()"},{"line_number":66,"context_line":"        self.protectable_registry.load_plugins()"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"        if hasattr(self._config.provider, \u0027bank\u0027) \\"},{"line_number":69,"context_line":"                and not self._config.provider.bank:"},{"line_number":70,"context_line":"            raise ImportError(\"Empty bank\")"},{"line_number":71,"context_line":"        self._load_bank(self._config.provider.bank)"},{"line_number":72,"context_line":"        if hasattr(self._config.provider, \u0027plugin\u0027):"}],"source_content_type":"text/x-python","patch_set":4,"id":"dab17558_cbbaf209","line":69,"range":{"start_line":68,"start_character":0,"end_line":69,"end_character":51},"updated":"2016-05-10 03:29:34.000000000","message":"should be:\nif not hasattr(self._config.provider, \u0027bank\u0027) \\\n    or not self._config.provider.bank:","commit_id":"faed7d12de49740dfe7f6f851609b29d3f42f46b"},{"author":{"_account_id":18266,"name":"zengchen","email":"chenzeng2@huawei.com","username":"zengchen"},"change_message_id":"b30a2f7c43efeedd3ce4da9104fd5679cea2f1f0","unresolved":false,"context_lines":[{"line_number":75,"context_line":"                    raise ImportError(\"Empty protection plugin\")"},{"line_number":76,"context_line":"                self._load_plugin(plugin_name)"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"        if self._bank_plugin:"},{"line_number":79,"context_line":"            self.checkpoint_collection \u003d checkpoint.CheckpointCollection("},{"line_number":80,"context_line":"                self._bank_plugin)"},{"line_number":81,"context_line":"        else:"},{"line_number":82,"context_line":"            LOG.error(_LE(\u0027Bank plugin not exist, check your configuration\u0027))"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    @property"},{"line_number":85,"context_line":"    def id(self):"}],"source_content_type":"text/x-python","patch_set":4,"id":"dab17558_cba9527d","line":82,"range":{"start_line":78,"start_character":0,"end_line":82,"end_character":77},"updated":"2016-05-10 03:29:34.000000000","message":"1. No need check self._bank_plugin, because self._load_bank will add a value to self._bank_plugin if no exceptions happen.\n2. Put these codes after \u0027self._load_bank(self._config.provider.bank)\u0027 in order to make the logic more compact","commit_id":"faed7d12de49740dfe7f6f851609b29d3f42f46b"},{"author":{"_account_id":18266,"name":"zengchen","email":"chenzeng2@huawei.com","username":"zengchen"},"change_message_id":"b30a2f7c43efeedd3ce4da9104fd5679cea2f1f0","unresolved":false,"context_lines":[{"line_number":124,"context_line":"            raise"},{"line_number":125,"context_line":"        else:"},{"line_number":126,"context_line":"            self._plugin_map[plugin_name] \u003d plugin"},{"line_number":127,"context_line":"            for resource in plugin.get_supported_resources_types():"},{"line_number":128,"context_line":"                if hasattr(plugin, \u0027get_options_schema\u0027):"},{"line_number":129,"context_line":"                    self._extended_info_schema[\u0027options_schema\u0027][resource] \\"},{"line_number":130,"context_line":"                        \u003d plugin.get_options_schema(resource)"}],"source_content_type":"text/x-python","patch_set":4,"id":"dab17558_2b057e79","line":127,"range":{"start_line":127,"start_character":35,"end_line":127,"end_character":64},"updated":"2016-05-10 03:29:34.000000000","message":"do we need to avoid two different plugins which support same resource type to exist in one provider?","commit_id":"faed7d12de49740dfe7f6f851609b29d3f42f46b"},{"author":{"_account_id":18266,"name":"zengchen","email":"chenzeng2@huawei.com","username":"zengchen"},"change_message_id":"90d17f318d85c7fefb4d8ebdf8de57b359f16668","unresolved":false,"context_lines":[{"line_number":138,"context_line":"    def get_checkpoint_collection(self):"},{"line_number":139,"context_line":"        return self.checkpoint_collection"},{"line_number":140,"context_line":""},{"line_number":141,"context_line":"    def build_task_flow(self, operation_type, context, workflow_engine,"},{"line_number":142,"context_line":"                        resource_graph, parameters):"},{"line_number":143,"context_line":"        # TODO(yuvalbr)"},{"line_number":144,"context_line":"        pass"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"class ProviderRegistry(object):"}],"source_content_type":"text/x-python","patch_set":4,"id":"dab17558_863621e5","line":144,"range":{"start_line":141,"start_character":0,"end_line":144,"end_character":12},"updated":"2016-05-10 03:53:48.000000000","message":"IMO, the logic of \u0027build_task_flow\u0027 should not be put here, because it is not the responsibility of PluggableProtectionProvider.","commit_id":"faed7d12de49740dfe7f6f851609b29d3f42f46b"},{"author":{"_account_id":18266,"name":"zengchen","email":"chenzeng2@huawei.com","username":"zengchen"},"change_message_id":"f4ae42e73e789124d853ae28ede8819b08f960a6","unresolved":false,"context_lines":[{"line_number":138,"context_line":"    def get_checkpoint_collection(self):"},{"line_number":139,"context_line":"        return self.checkpoint_collection"},{"line_number":140,"context_line":""},{"line_number":141,"context_line":"    def build_task_flow(self, operation_type, context, workflow_engine,"},{"line_number":142,"context_line":"                        resource_graph, parameters):"},{"line_number":143,"context_line":"        # TODO(yuvalbr)"},{"line_number":144,"context_line":"        pass"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"class ProviderRegistry(object):"}],"source_content_type":"text/x-python","patch_set":4,"id":"dab17558_667dfd6d","line":144,"range":{"start_line":141,"start_character":0,"end_line":144,"end_character":12},"in_reply_to":"dab17558_863621e5","updated":"2016-05-10 03:58:10.000000000","message":"Maybe should move to the module where generating the flow, like the \u0027create_protection.py\u0027","commit_id":"faed7d12de49740dfe7f6f851609b29d3f42f46b"}]}