)]}'
{"tempest/cmd/workspace.py":[{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"826309873b841a0e8c0cf2abba95bf3d42380eee","unresolved":false,"context_lines":[{"line_number":107,"context_line":"        self._populate()"},{"line_number":108,"context_line":"        self._name_exists(name)"},{"line_number":109,"context_line":"        workspace_path \u003d self.workspaces.pop(name)"},{"line_number":110,"context_line":"        shutil.rmtree(workspace_path)"},{"line_number":111,"context_line":"        self._write_file()"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"    @lockutils.synchronized(\u0027workspaces\u0027, external\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f201791_4664454a","line":110,"updated":"2017-06-26 14:11:11.000000000","message":"This needs to be an additional flag. Besides this being a huge backwards incompatible change to the cli it also probably isn\u0027t a good idea to default to deleting everything.","commit_id":"7082bf68054bd8621745612ea1061c0946d175ea"},{"author":{"_account_id":12393,"name":"chandan kumar","display_name":"Chandan Kumar","email":"chkumar@redhat.com","username":"chkumar246"},"change_message_id":"201c204885798f1abd9614ba5e1a4068e1bc83d0","unresolved":false,"context_lines":[{"line_number":107,"context_line":"        self._populate()"},{"line_number":108,"context_line":"        self._name_exists(name)"},{"line_number":109,"context_line":"        workspace_path \u003d self.workspaces.pop(name)"},{"line_number":110,"context_line":"        shutil.rmtree(workspace_path)"},{"line_number":111,"context_line":"        self._write_file()"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"    @lockutils.synchronized(\u0027workspaces\u0027, external\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f201791_45489b85","line":110,"in_reply_to":"5f201791_4664454a","updated":"2017-06-27 11:43:38.000000000","message":"Done","commit_id":"7082bf68054bd8621745612ea1061c0946d175ea"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"ba42e71c55e02ad8b17e73a885531bf9657f0071","unresolved":false,"context_lines":[{"line_number":105,"context_line":"            sys.exit(1)"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    @lockutils.synchronized(\u0027workspaces\u0027, external\u003dTrue)"},{"line_number":108,"context_line":"    def remove_workspace(self, name):"},{"line_number":109,"context_line":"        self._populate()"},{"line_number":110,"context_line":"        self._name_exists(name)"},{"line_number":111,"context_line":"        self.workspaces.pop(name)"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f201791_5b6aad19","line":108,"range":{"start_line":108,"start_character":8,"end_line":108,"end_character":24},"updated":"2017-06-27 18:15:53.000000000","message":"TBH, I cannot understand the difference between \"remove\" and \"delete\" on this context. So how about re-defining this existing method as \"remove_workspace_entry\" ?\n\nAnd how about defining the following new method as \"remove_workspace_directory\" ?","commit_id":"d32788d64a155a99aaf06bf5961f9e7dfa41ced9"},{"author":{"_account_id":10644,"name":"David Paterson","email":"davpat2112@yahoo.com","username":"dpaterson"},"change_message_id":"fb0dc0d7bf2055e65e34f51ee81a1504bdfb1762","unresolved":false,"context_lines":[{"line_number":105,"context_line":"            sys.exit(1)"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    @lockutils.synchronized(\u0027workspaces\u0027, external\u003dTrue)"},{"line_number":108,"context_line":"    def remove_workspace(self, name):"},{"line_number":109,"context_line":"        self._populate()"},{"line_number":110,"context_line":"        self._name_exists(name)"},{"line_number":111,"context_line":"        self.workspaces.pop(name)"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f201791_5b5d0d45","line":108,"range":{"start_line":108,"start_character":8,"end_line":108,"end_character":24},"in_reply_to":"5f201791_5b6aad19","updated":"2017-06-27 18:27:01.000000000","message":"I concur with Ken\u0027ichi, the difference between remove and delete\u0027s objectives are blurry.","commit_id":"d32788d64a155a99aaf06bf5961f9e7dfa41ced9"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"ba42e71c55e02ad8b17e73a885531bf9657f0071","unresolved":false,"context_lines":[{"line_number":236,"context_line":"        parser \u003d super(TempestWorkspaceRemove, self).get_parser(prog_name)"},{"line_number":237,"context_line":"        add_global_arguments(parser)"},{"line_number":238,"context_line":"        parser.add_argument(\u0027--name\u0027, required\u003dTrue)"},{"line_number":239,"context_line":"        parser.add_argument(\u0027--delete\u0027, \u0027-d\u0027, action\u003d\u0027store_true\u0027,"},{"line_number":240,"context_line":"                            help\u003d\u0027Deletes the given workspace directory\u0027)"},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"        return parser"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f201791_9b5de57a","line":239,"range":{"start_line":239,"start_character":28,"end_line":239,"end_character":38},"updated":"2017-06-27 18:15:53.000000000","message":"As the same above comment, how about \"--with-directory\" or something?","commit_id":"d32788d64a155a99aaf06bf5961f9e7dfa41ced9"},{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"38c86cca74b5223f9e2d35e6498223d541cb22de","unresolved":false,"context_lines":[{"line_number":236,"context_line":"        parser \u003d super(TempestWorkspaceRemove, self).get_parser(prog_name)"},{"line_number":237,"context_line":"        add_global_arguments(parser)"},{"line_number":238,"context_line":"        parser.add_argument(\u0027--name\u0027, required\u003dTrue)"},{"line_number":239,"context_line":"        parser.add_argument(\u0027--delete\u0027, \u0027-d\u0027, action\u003d\u0027store_true\u0027,"},{"line_number":240,"context_line":"                            help\u003d\u0027Deletes the given workspace directory\u0027)"},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"        return parser"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f201791_5baf4d29","line":239,"range":{"start_line":239,"start_character":28,"end_line":239,"end_character":38},"in_reply_to":"5f201791_9b5de57a","updated":"2017-06-27 18:24:26.000000000","message":"I\u0027m fine with changing the internal function names, but --with-directory is actual an ambiguous flag. Especially if you don\u0027t have the full context on the internals. Maybe --delete-directory would be a clearer name for you.","commit_id":"d32788d64a155a99aaf06bf5961f9e7dfa41ced9"},{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"38c86cca74b5223f9e2d35e6498223d541cb22de","unresolved":false,"context_lines":[{"line_number":246,"context_line":"        if parsed_args.delete:"},{"line_number":247,"context_line":"            self.manager.delete_workspace(parsed_args.name)"},{"line_number":248,"context_line":"            self.manager.remove_workspace(parsed_args.name)"},{"line_number":249,"context_line":"        else:"},{"line_number":250,"context_line":"            self.manager.remove_workspace(parsed_args.name)"},{"line_number":251,"context_line":"        sys.exit(0)"},{"line_number":252,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"5f201791_1ba55547","line":249,"updated":"2017-06-27 18:24:26.000000000","message":"You don\u0027t need to branch like this, we always remove the workspace. You should just do::\n\n  self.manager.remove_workspace(parsed_args.name)\n  if parsed_args.delete:\n      self.manager.delete_workspace(parsed_args.name)","commit_id":"d32788d64a155a99aaf06bf5961f9e7dfa41ced9"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"0c14c077aef79b44b2f270e9548d8278720db40c","unresolved":false,"context_lines":[{"line_number":246,"context_line":"        if parsed_args.delete:"},{"line_number":247,"context_line":"            self.manager.delete_workspace(parsed_args.name)"},{"line_number":248,"context_line":"            self.manager.remove_workspace(parsed_args.name)"},{"line_number":249,"context_line":"        else:"},{"line_number":250,"context_line":"            self.manager.remove_workspace(parsed_args.name)"},{"line_number":251,"context_line":"        sys.exit(0)"},{"line_number":252,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"5f201791_5e673bea","line":249,"in_reply_to":"5f201791_1ba55547","updated":"2017-06-27 18:35:12.000000000","message":"nice point, better code for me.","commit_id":"d32788d64a155a99aaf06bf5961f9e7dfa41ced9"},{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"7f88241d03f49f2027c45f5869509755424dc1b6","unresolved":false,"context_lines":[{"line_number":244,"context_line":"    def take_action(self, parsed_args):"},{"line_number":245,"context_line":"        self.manager \u003d WorkspaceManager(parsed_args.workspace_path)"},{"line_number":246,"context_line":"        if parsed_args.delete_directory:"},{"line_number":247,"context_line":"            self.manager.remove_workspace_directory(parsed_args.name)"},{"line_number":248,"context_line":"        self.manager.remove_workspace_entry(parsed_args.name)"},{"line_number":249,"context_line":"        sys.exit(0)"},{"line_number":250,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5f201791_627c7a71","line":247,"updated":"2017-06-28 13:44:38.000000000","message":"I actually think this should happen second. We don\u0027t ever want to be in a situation where we\u0027ve deleted the directory but failed to remove the workspace entry. It\u0027s an unlikely failure mode but something we should just protect against.","commit_id":"c859719a6081035f91b564d27fb33263bee44135"},{"author":{"_account_id":12393,"name":"chandan kumar","display_name":"Chandan Kumar","email":"chkumar@redhat.com","username":"chkumar246"},"change_message_id":"47399c5eead969b8bf872d18e95de0a859832480","unresolved":false,"context_lines":[{"line_number":244,"context_line":"    def take_action(self, parsed_args):"},{"line_number":245,"context_line":"        self.manager \u003d WorkspaceManager(parsed_args.workspace_path)"},{"line_number":246,"context_line":"        if parsed_args.delete_directory:"},{"line_number":247,"context_line":"            self.manager.remove_workspace_directory(parsed_args.name)"},{"line_number":248,"context_line":"        self.manager.remove_workspace_entry(parsed_args.name)"},{"line_number":249,"context_line":"        sys.exit(0)"},{"line_number":250,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5f201791_a2917249","line":247,"in_reply_to":"5f201791_627c7a71","updated":"2017-06-28 13:51:23.000000000","message":"If we call first self.manager.remove_workspace_entry, then it will delete the entry of workspace from workspace.yaml, then  self.manager.remove_workspace_directory will be called, it again look for workspace.yaml which will be found empty as self.manager.remove_workspace_entry method already removed the entry and the --delete-directory will fail.","commit_id":"c859719a6081035f91b564d27fb33263bee44135"},{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"38669633932e6157fc3fa2be8361ca9cd70512c3","unresolved":false,"context_lines":[{"line_number":244,"context_line":"    def take_action(self, parsed_args):"},{"line_number":245,"context_line":"        self.manager \u003d WorkspaceManager(parsed_args.workspace_path)"},{"line_number":246,"context_line":"        if parsed_args.delete_directory:"},{"line_number":247,"context_line":"            self.manager.remove_workspace_directory(parsed_args.name)"},{"line_number":248,"context_line":"        self.manager.remove_workspace_entry(parsed_args.name)"},{"line_number":249,"context_line":"        sys.exit(0)"},{"line_number":250,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5f201791_653fe486","line":247,"in_reply_to":"5f201791_a2917249","updated":"2017-06-28 14:13:11.000000000","message":"We just need to store the workspace path as a temp variable before we remove the entry. The easiest way to do this would probably be to have remove_workspace_directory() return the popped value from self.workspaces and then just pass that into remove_workspace_directory as an argument. Then just look it up from that dict arg.\n\nThis also has the advantage that you don\u0027t end up reading the yaml file twice.","commit_id":"c859719a6081035f91b564d27fb33263bee44135"}],"tempest/tests/cmd/test_workspace.py":[{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"7f88241d03f49f2027c45f5869509755424dc1b6","unresolved":false,"context_lines":[{"line_number":130,"context_line":""},{"line_number":131,"context_line":"    def test_workspace_manager_remove_directory(self):"},{"line_number":132,"context_line":"        self.workspace_manager.remove_workspace_directory(self.name)"},{"line_number":133,"context_line":"        self.test_workspace_manager_remove_entry()"},{"line_number":134,"context_line":"        self.assertIsNone(self.workspace_manager.get_workspace(self.name))"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"    def test_path_expansion(self):"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f201791_62ca3aa1","line":133,"updated":"2017-06-28 13:44:38.000000000","message":"You shouldn\u0027t call another test method like this from inside a test method. While it will probably work it breaks the unit test model and isolation between tests we try to build.","commit_id":"c859719a6081035f91b564d27fb33263bee44135"}]}