)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"e921e39b0a7c3edb921b4637978599c70fc580b7","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This makes use of the mandatory option of WSME, to remove some of the"},{"line_number":10,"context_line":"custom validation code. The patch also includes a new attribute on"},{"line_number":11,"context_line":"Ports API object called nodes_uuid to store the UUID of the node that"},{"line_number":12,"context_line":"Port belongs to, once this field is set it magically converts the UUID"},{"line_number":13,"context_line":"to the numeric ID of the node and sets the node_id attribute to be"},{"line_number":14,"context_line":"used internally."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"AAAATn%2F%2BL%2Fs%3D","line":11,"updated":"2013-11-29 16:21:46.000000000","message":"typo nodes_uuid -\u003e node_uuid","commit_id":"1a896eccae1ba167c4a05440757e3572d0c9def1"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"73e8679286897de28c13de8fb8d4ae36c40d39ec","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This makes use of the mandatory option of WSME, to remove some of the"},{"line_number":10,"context_line":"custom validation code. The patch also includes a new attribute on"},{"line_number":11,"context_line":"Ports API object called nodes_uuid to store the UUID of the node that"},{"line_number":12,"context_line":"Port belongs to, once this field is set it magically converts the UUID"},{"line_number":13,"context_line":"to the numeric ID of the node and sets the node_id attribute to be"},{"line_number":14,"context_line":"used internally."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"AAAATn%2F%2BLQw%3D","line":11,"in_reply_to":"AAAATn%2F%2BL%2Fs%3D","updated":"2013-11-29 17:47:42.000000000","message":"Oops... thanks","commit_id":"1a896eccae1ba167c4a05440757e3572d0c9def1"}],"ironic/api/controllers/v1/base.py":[{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"3dae67cfa9a503481b5215c5a0f715f7c52d713e","unresolved":false,"context_lines":[{"line_number":40,"context_line":"                        if fields and k not in fields))"},{"line_number":41,"context_line":"        return cls(**obj_dict)"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    def unset_fields(self, excepting\u003dNone):"},{"line_number":44,"context_line":"        \"\"\"Unset fields so they don\u0027t appear in the message body."},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"        :excepting: A list of fields that won\u0027t be touched."}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAATn%2F%2Bxas%3D","line":43,"updated":"2013-11-20 13:26:44.000000000","message":"excepting\u003d[]?","commit_id":"15ea06dad89eb7cdf44c1b86d1f4395350d8719f"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"3f08534cb5c935fbe2c2b4eb5cfc6bdea67ba855","unresolved":false,"context_lines":[{"line_number":40,"context_line":"                        if fields and k not in fields))"},{"line_number":41,"context_line":"        return cls(**obj_dict)"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    def unset_fields(self, excepting\u003dNone):"},{"line_number":44,"context_line":"        \"\"\"Unset fields so they don\u0027t appear in the message body."},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"        :excepting: A list of fields that won\u0027t be touched."}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAATn%2F%2Bwdo%3D","line":43,"in_reply_to":"AAAATn%2F%2BxG0%3D","updated":"2013-11-20 15:57:27.000000000","message":"Yes, just like Max said mutable in defaults arguments are dangerous.\n\ne.g:\n\nIn [1]: def test(i, l\u003d[]):\n   ...:     l.append(i)\n   ...:     print l\n   ...:     \n\nIn [2]: test(1)\n[1]\n\nIn [3]: test(2)\n[1, 2]\n\nIn [4]: test(3)\n[1, 2, 3]","commit_id":"15ea06dad89eb7cdf44c1b86d1f4395350d8719f"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"fad1b0db4214e7ed750167d9308984f77ecab04f","unresolved":false,"context_lines":[{"line_number":40,"context_line":"                        if fields and k not in fields))"},{"line_number":41,"context_line":"        return cls(**obj_dict)"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    def unset_fields(self, excepting\u003dNone):"},{"line_number":44,"context_line":"        \"\"\"Unset fields so they don\u0027t appear in the message body."},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"        :excepting: A list of fields that won\u0027t be touched."}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAATn%2F%2BxG0%3D","line":43,"in_reply_to":"AAAATn%2F%2Bxas%3D","updated":"2013-11-20 13:42:15.000000000","message":"No it\u0027s correct since it\u0027s not a good practice to put mutable in func defaults (it will remain between calls)","commit_id":"15ea06dad89eb7cdf44c1b86d1f4395350d8719f"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"884117c43642d49cf9351e8547335edcb571da6d","unresolved":false,"context_lines":[{"line_number":43,"context_line":"    def unset_fields(self, excepting\u003dNone):"},{"line_number":44,"context_line":"        \"\"\"Unset fields so they don\u0027t appear in the message body."},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"        :excepting: A list of fields that won\u0027t be touched."},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"        \"\"\""},{"line_number":49,"context_line":"        if excepting is None:"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAATn%2F%2Bc%2BU%3D","line":46,"updated":"2013-11-25 22:02:08.000000000","message":"\":param excepting: A...\"","commit_id":"5caa2c798cf4f5e1c0bc45af948286bd76b75031"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"14fad3b815f60dbeb7115eccd2c41c03ebb5406a","unresolved":false,"context_lines":[{"line_number":43,"context_line":"    def unset_fields(self, excepting\u003dNone):"},{"line_number":44,"context_line":"        \"\"\"Unset fields so they don\u0027t appear in the message body."},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"        :excepting: A list of fields that won\u0027t be touched."},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"        \"\"\""},{"line_number":49,"context_line":"        if excepting is None:"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAATn%2F%2BeOM%3D","line":46,"updated":"2013-11-25 23:03:48.000000000","message":"Shouldn\u0027t this say:\n\n  :param excepting: A list of fields ...","commit_id":"5caa2c798cf4f5e1c0bc45af948286bd76b75031"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"4f9579527db290b9d68914c4aa6ee6c99dea414b","unresolved":false,"context_lines":[{"line_number":43,"context_line":"    def unset_fields(self, excepting\u003dNone):"},{"line_number":44,"context_line":"        \"\"\"Unset fields so they don\u0027t appear in the message body."},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"        :excepting: A list of fields that won\u0027t be touched."},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"        \"\"\""},{"line_number":49,"context_line":"        if excepting is None:"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAATn%2F%2BZ1M%3D","line":46,"in_reply_to":"AAAATn%2F%2BeOM%3D","updated":"2013-11-26 14:58:55.000000000","message":"thanks, done.","commit_id":"5caa2c798cf4f5e1c0bc45af948286bd76b75031"}],"ironic/api/controllers/v1/port.py":[{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"14fad3b815f60dbeb7115eccd2c41c03ebb5406a","unresolved":false,"context_lines":[{"line_number":51,"context_line":"        return self._node_uuid"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    def _set_node_uuid(self, value):"},{"line_number":54,"context_line":"        if value:"},{"line_number":55,"context_line":"            try:"},{"line_number":56,"context_line":"                node \u003d objects.Node.get_by_uuid(pecan.request.context, value)"},{"line_number":57,"context_line":"                self._node_uuid \u003d node.uuid"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAATn%2F%2BeM4%3D","line":54,"updated":"2013-11-25 23:03:48.000000000","message":"What if self.node_id is already set -- should it be allowed to change? should we check to see if the new value is same, and then avoid db call?","commit_id":"5caa2c798cf4f5e1c0bc45af948286bd76b75031"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"4f9579527db290b9d68914c4aa6ee6c99dea414b","unresolved":false,"context_lines":[{"line_number":51,"context_line":"        return self._node_uuid"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    def _set_node_uuid(self, value):"},{"line_number":54,"context_line":"        if value:"},{"line_number":55,"context_line":"            try:"},{"line_number":56,"context_line":"                node \u003d objects.Node.get_by_uuid(pecan.request.context, value)"},{"line_number":57,"context_line":"                self._node_uuid \u003d node.uuid"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAATn%2F%2BZ0c%3D","line":54,"in_reply_to":"AAAATn%2F%2BeM4%3D","updated":"2013-11-26 14:58:55.000000000","message":"I think we should be allowed it to change just like any other attribute in the class.\n\n+1 For checking if the new value is the same and avoid a db call, will change it. \n\nThanks","commit_id":"5caa2c798cf4f5e1c0bc45af948286bd76b75031"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"884117c43642d49cf9351e8547335edcb571da6d","unresolved":false,"context_lines":[{"line_number":60,"context_line":"                #                    conversion."},{"line_number":61,"context_line":"                self.node_id \u003d node.id"},{"line_number":62,"context_line":"            except exception.NodeNotFound as e:"},{"line_number":63,"context_line":"                e.code \u003d 400  # BadRequestt"},{"line_number":64,"context_line":"                raise e"},{"line_number":65,"context_line":"        elif value \u003d\u003d wtypes.Unset:"},{"line_number":66,"context_line":"            self._node_uuid \u003d wtypes.Unset"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAATn%2F%2Bc94%3D","line":63,"updated":"2013-11-25 22:02:08.000000000","message":"Typo - double t","commit_id":"5caa2c798cf4f5e1c0bc45af948286bd76b75031"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"14fad3b815f60dbeb7115eccd2c41c03ebb5406a","unresolved":false,"context_lines":[{"line_number":60,"context_line":"                #                    conversion."},{"line_number":61,"context_line":"                self.node_id \u003d node.id"},{"line_number":62,"context_line":"            except exception.NodeNotFound as e:"},{"line_number":63,"context_line":"                e.code \u003d 400  # BadRequestt"},{"line_number":64,"context_line":"                raise e"},{"line_number":65,"context_line":"        elif value \u003d\u003d wtypes.Unset:"},{"line_number":66,"context_line":"            self._node_uuid \u003d wtypes.Unset"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAATn%2F%2BeNY%3D","line":63,"updated":"2013-11-25 23:03:48.000000000","message":"typo - two \"t\"s\n\nAlso, it would be helpful to explain why we\u0027re converting e.code. Something like:\n\n  # change error code because 404NotFound is inappropriate\n  # response for a POST request to create a Port.","commit_id":"5caa2c798cf4f5e1c0bc45af948286bd76b75031"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"4f9579527db290b9d68914c4aa6ee6c99dea414b","unresolved":false,"context_lines":[{"line_number":60,"context_line":"                #                    conversion."},{"line_number":61,"context_line":"                self.node_id \u003d node.id"},{"line_number":62,"context_line":"            except exception.NodeNotFound as e:"},{"line_number":63,"context_line":"                e.code \u003d 400  # BadRequestt"},{"line_number":64,"context_line":"                raise e"},{"line_number":65,"context_line":"        elif value \u003d\u003d wtypes.Unset:"},{"line_number":66,"context_line":"            self._node_uuid \u003d wtypes.Unset"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAATn%2F%2BZzw%3D","line":63,"in_reply_to":"AAAATn%2F%2BeNY%3D","updated":"2013-11-26 14:58:55.000000000","message":"Ahh great thanks guys.\n\nIndeed it will be good to put some explanation why we are changing the return code like that.","commit_id":"5caa2c798cf4f5e1c0bc45af948286bd76b75031"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"14fad3b815f60dbeb7115eccd2c41c03ebb5406a","unresolved":false,"context_lines":[{"line_number":95,"context_line":"    def convert_with_links(cls, rpc_port, expand\u003dTrue):"},{"line_number":96,"context_line":"        port \u003d Port(**rpc_port.as_dict())"},{"line_number":97,"context_line":"        if not expand:"},{"line_number":98,"context_line":"            port.unset_fields(excepting\u003d[\u0027uuid\u0027, \u0027address\u0027])"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"        # never expose the node_id attribute"},{"line_number":101,"context_line":"        port.node_id \u003d wtypes.Unset"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAATn%2F%2BeI4%3D","line":98,"updated":"2013-11-25 23:03:48.000000000","message":"Reading this is not intuitive. I think this would be better:\n\n  if not expand:\n    unset_fields(port, excepting\u003d[\u0027uuid\u0027, \u0027address\u0027])\n\nor:\n\n  if not expand:\n    port.unset_fields_except([\u0027uuid\u0027, \u0027address\u0027])\n\nWhat do you think?","commit_id":"5caa2c798cf4f5e1c0bc45af948286bd76b75031"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"4f9579527db290b9d68914c4aa6ee6c99dea414b","unresolved":false,"context_lines":[{"line_number":95,"context_line":"    def convert_with_links(cls, rpc_port, expand\u003dTrue):"},{"line_number":96,"context_line":"        port \u003d Port(**rpc_port.as_dict())"},{"line_number":97,"context_line":"        if not expand:"},{"line_number":98,"context_line":"            port.unset_fields(excepting\u003d[\u0027uuid\u0027, \u0027address\u0027])"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"        # never expose the node_id attribute"},{"line_number":101,"context_line":"        port.node_id \u003d wtypes.Unset"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAATn%2F%2BZhw%3D","line":98,"in_reply_to":"AAAATn%2F%2BeI4%3D","updated":"2013-11-26 14:58:55.000000000","message":"Yea the current name can be a bit obscure. I think the second example port.unset_fields_except() looks more straight forward, cause the same already tells you that you could input a list of exceptions.\n\nI will go with this one, thanks for the suggestions","commit_id":"5caa2c798cf4f5e1c0bc45af948286bd76b75031"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"e921e39b0a7c3edb921b4637978599c70fc580b7","unresolved":false,"context_lines":[{"line_number":58,"context_line":"                # NOTE(lucasagomes): Create the node_id attribute on-the-fly"},{"line_number":59,"context_line":"                #                    to satisfy the api -\u003e rpc object"},{"line_number":60,"context_line":"                #                    conversion."},{"line_number":61,"context_line":"                self.node_id \u003d node.id"},{"line_number":62,"context_line":"            except exception.NodeNotFound as e:"},{"line_number":63,"context_line":"                # Change error code because 404 (NotFound) is inappropriate"},{"line_number":64,"context_line":"                # response for a POST request to create a Port"}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAATn%2F%2BL9E%3D","line":61,"updated":"2013-11-29 16:21:46.000000000","message":"So if node_uuid was never set it will be None, but node_id will throw AttributeError, am I right? Maybe then create a node_id \u003d None on class level just for consistent behavior with node_uuid","commit_id":"1a896eccae1ba167c4a05440757e3572d0c9def1"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"73e8679286897de28c13de8fb8d4ae36c40d39ec","unresolved":false,"context_lines":[{"line_number":58,"context_line":"                # NOTE(lucasagomes): Create the node_id attribute on-the-fly"},{"line_number":59,"context_line":"                #                    to satisfy the api -\u003e rpc object"},{"line_number":60,"context_line":"                #                    conversion."},{"line_number":61,"context_line":"                self.node_id \u003d node.id"},{"line_number":62,"context_line":"            except exception.NodeNotFound as e:"},{"line_number":63,"context_line":"                # Change error code because 404 (NotFound) is inappropriate"},{"line_number":64,"context_line":"                # response for a POST request to create a Port"}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAATn%2F%2BLRY%3D","line":61,"in_reply_to":"AAAATn%2F%2BL9E%3D","updated":"2013-11-29 17:47:42.000000000","message":"Hi Max,\n\nThe problem is that we don\u0027t want \u0027node_id\u0027 to exist in the API object. On the API we only want to expose UUIDs, and internally they can be converted to the numeric ID to do operation such as JOINs etc... (Where an UUID would be very slow). The reason why node_id is being created on the fly (once node_uuid is set) is because both RPC and API objects share a list of fields, and having a consistence on this list makes the conversion from one type to another very easy.","commit_id":"1a896eccae1ba167c4a05440757e3572d0c9def1"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"2566067fae873f1f57189b4f1819148879e34873","unresolved":false,"context_lines":[{"line_number":58,"context_line":"                # NOTE(lucasagomes): Create the node_id attribute on-the-fly"},{"line_number":59,"context_line":"                #                    to satisfy the api -\u003e rpc object"},{"line_number":60,"context_line":"                #                    conversion."},{"line_number":61,"context_line":"                self.node_id \u003d node.id"},{"line_number":62,"context_line":"            except exception.NodeNotFound as e:"},{"line_number":63,"context_line":"                # Change error code because 404 (NotFound) is inappropriate"},{"line_number":64,"context_line":"                # response for a POST request to create a Port"}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAATn%2F%2BLN8%3D","line":61,"in_reply_to":"AAAATn%2F%2BLRY%3D","updated":"2013-11-29 18:02:43.000000000","message":"Makes sence to me now. Thanks for the rationale!","commit_id":"1a896eccae1ba167c4a05440757e3572d0c9def1"}],"ironic/tests/api/test_ports.py":[{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"98f316ab40eb0186561a692b8f8994f19808f9cb","unresolved":false,"context_lines":[{"line_number":370,"context_line":"        pdict[\u0027address\u0027] \u003d pdict[\u0027address\u0027].replace(\u0027:\u0027, \u0027-\u0027)"},{"line_number":371,"context_line":"        self.assertRaises(webtest.app.AppError,"},{"line_number":372,"context_line":"                          self.delete,"},{"line_number":373,"context_line":"                          \u0027/ports/%s\u0027 % pdict[\u0027address\u0027])"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2B5%2Bk%3D","line":373,"updated":"2013-11-18 18:23:17.000000000","message":"ironic/api/controllers/v1/port.py Port class became complex with that built-in conversion from id to uuid, so maybe to create a TestPortObject class here to test that?","commit_id":"52cc1fb54c42112ddf94cadee762a8db9fa79fca"},{"author":{"_account_id":8968,"name":"Max Lobur","email":"max_lobur@outlook.com","username":"max_lobur"},"change_message_id":"902ebab6705c5803a57f15db57236d974dae6c19","unresolved":false,"context_lines":[{"line_number":370,"context_line":"        pdict[\u0027address\u0027] \u003d pdict[\u0027address\u0027].replace(\u0027:\u0027, \u0027-\u0027)"},{"line_number":371,"context_line":"        self.assertRaises(webtest.app.AppError,"},{"line_number":372,"context_line":"                          self.delete,"},{"line_number":373,"context_line":"                          \u0027/ports/%s\u0027 % pdict[\u0027address\u0027])"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2B2e0%3D","line":373,"in_reply_to":"AAAATn%2F%2B2zA%3D","updated":"2013-11-19 11:15:13.000000000","message":"Yea, testing within POST looks good to me too. It\u0027s some kind of behavior testing instead of implementation testing, which is even better because we don\u0027t care how it works inside\n\nThanks!","commit_id":"52cc1fb54c42112ddf94cadee762a8db9fa79fca"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"80a045dbf58904d8d59d0c6dde48fb84c39fc879","unresolved":false,"context_lines":[{"line_number":370,"context_line":"        pdict[\u0027address\u0027] \u003d pdict[\u0027address\u0027].replace(\u0027:\u0027, \u0027-\u0027)"},{"line_number":371,"context_line":"        self.assertRaises(webtest.app.AppError,"},{"line_number":372,"context_line":"                          self.delete,"},{"line_number":373,"context_line":"                          \u0027/ports/%s\u0027 % pdict[\u0027address\u0027])"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2B2zA%3D","line":373,"in_reply_to":"AAAATn%2F%2B5%2Bk%3D","updated":"2013-11-19 10:25:36.000000000","message":"Sounds good to test it, problem with testing the object directly instantiating it is that I will have to mock things like the pecan.request object and such, I think it would be better to test the conversion with a POST instead (the POST would test the real thing without mocking stuff, which sounds better).\n\nThoughts?\n\nI\u0027m going to add a test to stress the conversion, thanks for the suggestion.","commit_id":"52cc1fb54c42112ddf94cadee762a8db9fa79fca"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"14fad3b815f60dbeb7115eccd2c41c03ebb5406a","unresolved":false,"context_lines":[{"line_number":23,"context_line":"from ironic.tests.db import utils as dbutils"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"#NOTE(lucasagomes): When creating a port via API (POST)"},{"line_number":27,"context_line":"#                   we have to use node_uuid"},{"line_number":28,"context_line":"def POST_get_test_port(**kw):"},{"line_number":29,"context_line":"    port \u003d dbutils.get_test_port(**kw)"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAATn%2F%2Bcvg%3D","line":26,"updated":"2013-11-25 23:03:48.000000000","message":"nit: please follow # with a space, eg,\n\n  # NOTE(lucasagomes): ...\n\n\nhttp://www.python.org/dev/peps/pep-0008/#block-comments","commit_id":"5caa2c798cf4f5e1c0bc45af948286bd76b75031"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"4f9579527db290b9d68914c4aa6ee6c99dea414b","unresolved":false,"context_lines":[{"line_number":23,"context_line":"from ironic.tests.db import utils as dbutils"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"#NOTE(lucasagomes): When creating a port via API (POST)"},{"line_number":27,"context_line":"#                   we have to use node_uuid"},{"line_number":28,"context_line":"def POST_get_test_port(**kw):"},{"line_number":29,"context_line":"    port \u003d dbutils.get_test_port(**kw)"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAATn%2F%2BZgI%3D","line":26,"in_reply_to":"AAAATn%2F%2Bcvg%3D","updated":"2013-11-26 14:58:55.000000000","message":"oops, done :)","commit_id":"5caa2c798cf4f5e1c0bc45af948286bd76b75031"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"14fad3b815f60dbeb7115eccd2c41c03ebb5406a","unresolved":false,"context_lines":[{"line_number":25,"context_line":""},{"line_number":26,"context_line":"#NOTE(lucasagomes): When creating a port via API (POST)"},{"line_number":27,"context_line":"#                   we have to use node_uuid"},{"line_number":28,"context_line":"def POST_get_test_port(**kw):"},{"line_number":29,"context_line":"    port \u003d dbutils.get_test_port(**kw)"},{"line_number":30,"context_line":"    node \u003d dbutils.get_test_node()"},{"line_number":31,"context_line":"    port[\u0027node_id\u0027] \u003d None"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAATn%2F%2BeE4%3D","line":28,"updated":"2013-11-25 23:03:48.000000000","message":"method name should be lower case, even for \"POST\"","commit_id":"5caa2c798cf4f5e1c0bc45af948286bd76b75031"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"4f9579527db290b9d68914c4aa6ee6c99dea414b","unresolved":false,"context_lines":[{"line_number":25,"context_line":""},{"line_number":26,"context_line":"#NOTE(lucasagomes): When creating a port via API (POST)"},{"line_number":27,"context_line":"#                   we have to use node_uuid"},{"line_number":28,"context_line":"def POST_get_test_port(**kw):"},{"line_number":29,"context_line":"    port \u003d dbutils.get_test_port(**kw)"},{"line_number":30,"context_line":"    node \u003d dbutils.get_test_node()"},{"line_number":31,"context_line":"    port[\u0027node_id\u0027] \u003d None"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAATn%2F%2BZe4%3D","line":28,"in_reply_to":"AAAATn%2F%2BeE4%3D","updated":"2013-11-26 14:58:55.000000000","message":"done, thanks :)","commit_id":"5caa2c798cf4f5e1c0bc45af948286bd76b75031"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"14fad3b815f60dbeb7115eccd2c41c03ebb5406a","unresolved":false,"context_lines":[{"line_number":352,"context_line":""},{"line_number":353,"context_line":"    def test_create_port_non_existent_node_uuid(self):"},{"line_number":354,"context_line":"        pdict \u003d POST_get_test_port("},{"line_number":355,"context_line":"                node_id\u003dNone, node_uuid\u003d\u0027766947f3-0d91-42ef-9899-e44c3e59f494\u0027)"},{"line_number":356,"context_line":""},{"line_number":357,"context_line":"        response \u003d self.post_json(\u0027/ports\u0027, pdict, expect_errors\u003dTrue)"},{"line_number":358,"context_line":"        self.assertEqual(response.content_type, \u0027application/json\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAATn%2F%2BcuQ%3D","line":355,"updated":"2013-11-25 23:03:48.000000000","message":"where is this node_uuid constant string coming from? Is this particular value related to the error being tested? If it\u0027s just a fake or random uuid, please generate one. I think it\u0027s best if we don\u0027t put raw strings (that aren\u0027t obviously fake test data) in the unit test methods.","commit_id":"5caa2c798cf4f5e1c0bc45af948286bd76b75031"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"4f9579527db290b9d68914c4aa6ee6c99dea414b","unresolved":false,"context_lines":[{"line_number":352,"context_line":""},{"line_number":353,"context_line":"    def test_create_port_non_existent_node_uuid(self):"},{"line_number":354,"context_line":"        pdict \u003d POST_get_test_port("},{"line_number":355,"context_line":"                node_id\u003dNone, node_uuid\u003d\u0027766947f3-0d91-42ef-9899-e44c3e59f494\u0027)"},{"line_number":356,"context_line":""},{"line_number":357,"context_line":"        response \u003d self.post_json(\u0027/ports\u0027, pdict, expect_errors\u003dTrue)"},{"line_number":358,"context_line":"        self.assertEqual(response.content_type, \u0027application/json\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAATn%2F%2BZeA%3D","line":355,"in_reply_to":"AAAATn%2F%2BcuQ%3D","updated":"2013-11-26 14:58:55.000000000","message":"Yea it\u0027s a uuid that does not belongs to any node, so the test will fail cause the node won\u0027t be found. Right, I will use a fake UUID instead, generating sounds a bit obscure for this kinda of test. \n\nI will also change the name of this method, maybe test_create_port_node_uuid_not_found() sounds more clear to me.\n\nThanks","commit_id":"5caa2c798cf4f5e1c0bc45af948286bd76b75031"}],"ironic/tests/api/test_types.py":[{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"14fad3b815f60dbeb7115eccd2c41c03ebb5406a","unresolved":false,"context_lines":[{"line_number":21,"context_line":"from ironic.tests.api import base"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"class TestMacAddressType(base.FunctionalTest):"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    def test_valid_mac_addr(self):"},{"line_number":27,"context_line":"        types.MacAddressType.validate(\u00273c:a9:f4:34:bc:a8\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAATn%2F%2Bcrc%3D","line":24,"updated":"2013-11-25 23:03:48.000000000","message":"@lucas, I think this unit test should only test that ironic.common.utils:is_valid_mac() is being called, and that a failure of that method raises a ValueError. There is no need to duplicate the testing of that method here.\n\nDitto for the TestUuidType test case.","commit_id":"5caa2c798cf4f5e1c0bc45af948286bd76b75031"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"4f9579527db290b9d68914c4aa6ee6c99dea414b","unresolved":false,"context_lines":[{"line_number":21,"context_line":"from ironic.tests.api import base"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"class TestMacAddressType(base.FunctionalTest):"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    def test_valid_mac_addr(self):"},{"line_number":27,"context_line":"        types.MacAddressType.validate(\u00273c:a9:f4:34:bc:a8\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAATn%2F%2BZb8%3D","line":24,"in_reply_to":"AAAATn%2F%2Bcrc%3D","updated":"2013-11-26 14:58:55.000000000","message":"Yes makes sense, both tests seems to be redundant, I\u0027m going to drop remove them. Thanks","commit_id":"5caa2c798cf4f5e1c0bc45af948286bd76b75031"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"884117c43642d49cf9351e8547335edcb571da6d","unresolved":false,"context_lines":[{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    def test_invalid_mac_addr_hyphen(self):"},{"line_number":34,"context_line":"        self.assertRaises(ValueError,"},{"line_number":35,"context_line":"                          types.MacAddressType.validate, \u0027AA-AA-AA-AA-AA-AA\u0027)"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"class TestUuidType(base.FunctionalTest):"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAATn%2F%2Bc9I%3D","line":35,"updated":"2013-11-25 22:02:08.000000000","message":"Do we want to make this invalid? Or accept and translate?\n\nSecondly, is 3C:A9:F4:34:BC:A8 a valid mac? If so, please add a test.. and if not likewise :)","commit_id":"5caa2c798cf4f5e1c0bc45af948286bd76b75031"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"14fad3b815f60dbeb7115eccd2c41c03ebb5406a","unresolved":false,"context_lines":[{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    def test_invalid_mac_addr_hyphen(self):"},{"line_number":34,"context_line":"        self.assertRaises(ValueError,"},{"line_number":35,"context_line":"                          types.MacAddressType.validate, \u0027AA-AA-AA-AA-AA-AA\u0027)"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"class TestUuidType(base.FunctionalTest):"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAATn%2F%2BctU%3D","line":35,"in_reply_to":"AAAATn%2F%2Bc9I%3D","updated":"2013-11-25 23:03:48.000000000","message":"@lifeless, silent conversions are dangerous, so we reject hyphenated macs rather than translate.","commit_id":"5caa2c798cf4f5e1c0bc45af948286bd76b75031"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"4f9579527db290b9d68914c4aa6ee6c99dea414b","unresolved":false,"context_lines":[{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    def test_invalid_mac_addr_hyphen(self):"},{"line_number":34,"context_line":"        self.assertRaises(ValueError,"},{"line_number":35,"context_line":"                          types.MacAddressType.validate, \u0027AA-AA-AA-AA-AA-AA\u0027)"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"class TestUuidType(base.FunctionalTest):"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAATn%2F%2BZaU%3D","line":35,"in_reply_to":"AAAATn%2F%2BctU%3D","updated":"2013-11-26 14:58:55.000000000","message":"Right as pointed by devananda it\u0027s better to test the is_valid_mac() function directly because all this validate function does is to call that method.","commit_id":"5caa2c798cf4f5e1c0bc45af948286bd76b75031"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"884117c43642d49cf9351e8547335edcb571da6d","unresolved":false,"context_lines":[{"line_number":38,"context_line":"class TestUuidType(base.FunctionalTest):"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    def test_valid_uuid(self):"},{"line_number":41,"context_line":"        types.UuidType.validate(\u002794e13bc3-5601-412d-8cec-6e93d841250a\u0027)"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    def test_invalid_mac_address(self):"},{"line_number":44,"context_line":"        self.assertRaises(ValueError,"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAATn%2F%2Bc9E%3D","line":41,"updated":"2013-11-25 22:02:08.000000000","message":"same here - case checking.","commit_id":"5caa2c798cf4f5e1c0bc45af948286bd76b75031"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"4f9579527db290b9d68914c4aa6ee6c99dea414b","unresolved":false,"context_lines":[{"line_number":38,"context_line":"class TestUuidType(base.FunctionalTest):"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    def test_valid_uuid(self):"},{"line_number":41,"context_line":"        types.UuidType.validate(\u002794e13bc3-5601-412d-8cec-6e93d841250a\u0027)"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    def test_invalid_mac_address(self):"},{"line_number":44,"context_line":"        self.assertRaises(ValueError,"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAATn%2F%2BZaw%3D","line":41,"in_reply_to":"AAAATn%2F%2Bc9E%3D","updated":"2013-11-26 14:58:55.000000000","message":"I will remove this tests since all it does is to call uuidutils.is_uuid_list() from oslo, and that function is already being tested","commit_id":"5caa2c798cf4f5e1c0bc45af948286bd76b75031"}]}