)]}'
{"config_tempest/flavors.py":[{"author":{"_account_id":10459,"name":"Luigi Toscano","email":"ltoscano@redhat.com","username":"ltoscano"},"change_message_id":"e7baf392464f1a6377a9d77516b7523811e79657","unresolved":false,"context_lines":[{"line_number":41,"context_line":"        if self._conf.has_option(\u0027compute\u0027, \u0027flavor_ref\u0027):"},{"line_number":42,"context_line":"            flavor_id \u003d self._conf.get(\u0027compute\u0027, \u0027flavor_ref\u0027)"},{"line_number":43,"context_line":"            flavor_id \u003d self.find_flavor_by_id(flavor_id)"},{"line_number":44,"context_line":"            if flavor_id is None:"},{"line_number":45,"context_line":"                raise Exception(\"flavor_ref id \u0027%s\u0027 specified by user doesn\u0027t\""},{"line_number":46,"context_line":"                                \" exist\", flavor_id)"},{"line_number":47,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":5,"id":"5f7c97a3_32d8de7e","line":44,"updated":"2018-07-13 08:43:43.000000000","message":"The code for each flavor is almost the same. While previously it was only 3 lines, now the check for the existence of each flavor is made of more lines which are mostly the same.\nCan those lines be consolidated inside create_flavor, which could become for example find_or_create_flavor ?","commit_id":"e6c6f686e456a576c61db3e3158340603dd80c92"},{"author":{"_account_id":22873,"name":"Martin Kopec","email":"mkopec@redhat.com","username":"mkopec"},"change_message_id":"c7e71a6d976c6279d5d283de91a76a37ef5e0b5e","unresolved":false,"context_lines":[{"line_number":41,"context_line":"        if self._conf.has_option(\u0027compute\u0027, \u0027flavor_ref\u0027):"},{"line_number":42,"context_line":"            flavor_id \u003d self._conf.get(\u0027compute\u0027, \u0027flavor_ref\u0027)"},{"line_number":43,"context_line":"            flavor_id \u003d self.find_flavor_by_id(flavor_id)"},{"line_number":44,"context_line":"            if flavor_id is None:"},{"line_number":45,"context_line":"                raise Exception(\"flavor_ref id \u0027%s\u0027 specified by user doesn\u0027t\""},{"line_number":46,"context_line":"                                \" exist\", flavor_id)"},{"line_number":47,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":5,"id":"5f7c97a3_86b3b39a","line":44,"in_reply_to":"5f7c97a3_32d8de7e","updated":"2018-07-13 10:14:27.000000000","message":"Before the change, the tool tried to find a flavor by an ID which was provided by a user and if the flavor didn\u0027t exist, it tried to create nano or micro flavor. I find this wrong, because a user provided the ID for a reason and if the flavor doesn\u0027t exist, the user should be notified and the problem shouldn\u0027t be covered by creating a different flavor.\n\nTherefor I moved the code for finding a flavor to an independent method.\n\nHowever, what I will do is to remove the code for finding a flavor by its name from else branches (the case when no flavor ID is specified by a user) because the create_flavor method checks if the flavor doesn\u0027t exist before it tries to create it.","commit_id":"e6c6f686e456a576c61db3e3158340603dd80c92"},{"author":{"_account_id":10459,"name":"Luigi Toscano","email":"ltoscano@redhat.com","username":"ltoscano"},"change_message_id":"8bb5a1a188d377946208f1f1bdd109c1778e85ab","unresolved":false,"context_lines":[{"line_number":41,"context_line":"        if self._conf.has_option(\u0027compute\u0027, \u0027flavor_ref\u0027):"},{"line_number":42,"context_line":"            flavor_id \u003d self._conf.get(\u0027compute\u0027, \u0027flavor_ref\u0027)"},{"line_number":43,"context_line":"            flavor_id \u003d self.find_flavor_by_id(flavor_id)"},{"line_number":44,"context_line":"            if flavor_id is None:"},{"line_number":45,"context_line":"                raise Exception(\"flavor_ref id \u0027%s\u0027 specified by user doesn\u0027t\""},{"line_number":46,"context_line":"                                \" exist\", flavor_id)"},{"line_number":47,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":5,"id":"5f7c97a3_e99358fe","line":44,"in_reply_to":"5f7c97a3_86b3b39a","updated":"2018-07-13 11:16:26.000000000","message":"I didn\u0027t explain it properly.\n\nThe point is that lines 40..54 and 56..70 are basically the same. The only difference is the name of the flavor, the key that identifies the flavor and the amount of required RAM.\n\nCan they be put together in a new method? What I proposed previously is to extend create_flavor to handle all the \"does it exist? No, then try to find, then create\" instead of a new method that then calls create_flavor. But no strong opinion on this. My main point is that those blocks of 15 lines are the same. Previously they were only 4 lines repeated, so no need for a new function but now the code is more complex.","commit_id":"e6c6f686e456a576c61db3e3158340603dd80c92"},{"author":{"_account_id":22873,"name":"Martin Kopec","email":"mkopec@redhat.com","username":"mkopec"},"change_message_id":"016c7e86ca746875c92cd83adb7c6746cd0d3201","unresolved":false,"context_lines":[{"line_number":41,"context_line":"        if self._conf.has_option(\u0027compute\u0027, \u0027flavor_ref\u0027):"},{"line_number":42,"context_line":"            flavor_id \u003d self._conf.get(\u0027compute\u0027, \u0027flavor_ref\u0027)"},{"line_number":43,"context_line":"            flavor_id \u003d self.find_flavor_by_id(flavor_id)"},{"line_number":44,"context_line":"            if flavor_id is None:"},{"line_number":45,"context_line":"                raise Exception(\"flavor_ref id \u0027%s\u0027 specified by user doesn\u0027t\""},{"line_number":46,"context_line":"                                \" exist\", flavor_id)"},{"line_number":47,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":5,"id":"5f7c97a3_9f97763a","line":44,"in_reply_to":"5f7c97a3_e99358fe","updated":"2018-07-13 12:50:47.000000000","message":"I see. But I don\u0027t like the idea of moving that to create_flavor method. Shorter and aimed for one purpose methods are easier testable. I\u0027m thinking about other ways how not repeat the code.","commit_id":"e6c6f686e456a576c61db3e3158340603dd80c92"},{"author":{"_account_id":10459,"name":"Luigi Toscano","email":"ltoscano@redhat.com","username":"ltoscano"},"change_message_id":"e7baf392464f1a6377a9d77516b7523811e79657","unresolved":false,"context_lines":[{"line_number":139,"context_line":"            f \u003d self.client.show_flavor(flavor[\u0027id\u0027])[\u0027flavor\u0027]"},{"line_number":140,"context_line":"            flavors.append((f[\u0027name\u0027], f[\u0027id\u0027], f[\u0027ram\u0027],"},{"line_number":141,"context_line":"                            f[\u0027disk\u0027], f[\u0027vcpus\u0027]))"},{"line_number":142,"context_line":"        # order by ram size"},{"line_number":143,"context_line":"        flavors.sort(key\u003dlambda tup: tup[2])"},{"line_number":144,"context_line":"        # take first three and sort by disk size"},{"line_number":145,"context_line":"        flavors \u003d flavors[:3]"}],"source_content_type":"text/x-python","patch_set":5,"id":"5f7c97a3_d235eaa8","line":142,"updated":"2018-07-13 08:43:43.000000000","message":"Maybe a multi-ordering in one shot? https://docs.python.org/2/howto/sorting.html#operator-module-functions","commit_id":"e6c6f686e456a576c61db3e3158340603dd80c92"},{"author":{"_account_id":22873,"name":"Martin Kopec","email":"mkopec@redhat.com","username":"mkopec"},"change_message_id":"c7e71a6d976c6279d5d283de91a76a37ef5e0b5e","unresolved":false,"context_lines":[{"line_number":139,"context_line":"            f \u003d self.client.show_flavor(flavor[\u0027id\u0027])[\u0027flavor\u0027]"},{"line_number":140,"context_line":"            flavors.append((f[\u0027name\u0027], f[\u0027id\u0027], f[\u0027ram\u0027],"},{"line_number":141,"context_line":"                            f[\u0027disk\u0027], f[\u0027vcpus\u0027]))"},{"line_number":142,"context_line":"        # order by ram size"},{"line_number":143,"context_line":"        flavors.sort(key\u003dlambda tup: tup[2])"},{"line_number":144,"context_line":"        # take first three and sort by disk size"},{"line_number":145,"context_line":"        flavors \u003d flavors[:3]"}],"source_content_type":"text/x-python","patch_set":5,"id":"5f7c97a3_292cf091","line":142,"in_reply_to":"5f7c97a3_d235eaa8","updated":"2018-07-13 10:14:27.000000000","message":"Done","commit_id":"e6c6f686e456a576c61db3e3158340603dd80c92"}]}