)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"ae52916d10d51074d6ae39718c2257717fda6952","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Currently it is possible to assign multiple"},{"line_number":10,"context_line":"gateway ports to a router. This patch add a validator"},{"line_number":11,"context_line":"to prevent this behavior and also allow for any further"},{"line_number":12,"context_line":"validations based on the device owners."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: I1a42576f183271ad04f62b0b8ab25a813fd3b2cf"},{"line_number":15,"context_line":"Partial-Bug: #1430345"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"7a77a97e_e3745537","line":12,"updated":"2016-11-17 07:07:11.000000000","message":"i don\u0027t understand why this validator can prevent a user from creating multiple gw ports. can you explain?","commit_id":"45ed93c57623c7849eda10c70b4b6c42e4f8759d"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"fc4462e2260a268a31b31ab55d08fc2f19e5b03a","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Currently it is possible to assign multiple"},{"line_number":10,"context_line":"gateway ports to a router. This patch add a validator"},{"line_number":11,"context_line":"to prevent this behavior and also allow for any further"},{"line_number":12,"context_line":"validations based on the device owners."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: I1a42576f183271ad04f62b0b8ab25a813fd3b2cf"},{"line_number":15,"context_line":"Partial-Bug: #1430345"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"7a77a97e_218c857e","line":12,"in_reply_to":"7a77a97e_6610535a","updated":"2016-11-17 08:04:06.000000000","message":"how this validator prevents it?","commit_id":"45ed93c57623c7849eda10c70b4b6c42e4f8759d"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"c926bd878bc92e028475f7e1418d233ad665f160","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Currently it is possible to assign multiple"},{"line_number":10,"context_line":"gateway ports to a router. This patch add a validator"},{"line_number":11,"context_line":"to prevent this behavior and also allow for any further"},{"line_number":12,"context_line":"validations based on the device owners."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: I1a42576f183271ad04f62b0b8ab25a813fd3b2cf"},{"line_number":15,"context_line":"Partial-Bug: #1430345"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"7a77a97e_6610535a","line":12,"in_reply_to":"7a77a97e_e3745537","updated":"2016-11-17 07:57:00.000000000","message":"As mentioned in the Bug report\n\n  The manually created port can be assign as a \"Device Owner\" \u003d network:router_gateway to a router that already have a default gateway which cause one router to have two default gateways.","commit_id":"45ed93c57623c7849eda10c70b4b6c42e4f8759d"},{"author":{"_account_id":5367,"name":"boden","email":"bodenvmw@gmail.com","username":"boden"},"change_message_id":"0c7ec1977f6cb056344d4edaf67bd72231440c32","unresolved":false,"context_lines":[{"line_number":10,"context_line":"gateway ports to a router. This patch add a validator"},{"line_number":11,"context_line":"to prevent this behavior and also allow for any further"},{"line_number":12,"context_line":"validations based on the device owners."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: I1a42576f183271ad04f62b0b8ab25a813fd3b2cf"},{"line_number":15,"context_line":"Partial-Bug: #1430345"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":19,"id":"7a77a97e_a2ec45f4","line":13,"updated":"2016-11-22 19:20:18.000000000","message":"BTW: where is this validator going to be used (which subprojects)?","commit_id":"239e1c40c186f5054d92199f75a1bce671b5457d"}],"neutron_lib/api/validators.py":[{"author":{"_account_id":5367,"name":"boden","email":"bodenvmw@gmail.com","username":"boden"},"change_message_id":"cc21fb7eae3f2e53420fc19b847f115f37ba8ea0","unresolved":false,"context_lines":[{"line_number":848,"context_line":"            segmentations[segmentation_type].add(segmentation_id)"},{"line_number":849,"context_line":""},{"line_number":850,"context_line":""},{"line_number":851,"context_line":"def validate_device_owner(data, valid_values\u003dNone):"},{"line_number":852,"context_line":"    msg \u003d validate_string(data, max_len\u003dconstants.DEVICE_OWNER_MAX_LEN)"},{"line_number":853,"context_line":"    if data \u003d\u003d constants.DEVICE_OWNER_ROUTER_GW:"},{"line_number":854,"context_line":"        msg \u003d _(\"Ports of type: \u0027%s\u0027 cannot be created by via API\") % data"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a98d1f4_e6922f4b","line":851,"range":{"start_line":851,"start_character":0,"end_line":851,"end_character":50},"updated":"2016-10-04 13:56:42.000000000","message":"A few comments:\n- Why not accept valid_values and check it like: https://github.com/openstack/neutron-lib/blob/master/neutron_lib/api/validators.py#L217\n- It would be great if you could add some docstrings for this function like all others in this file.\n- It seems like this validator should be more generic, right now it just checks that data is a string and is not constants.DEVICE_OWNER_ROUTER_GW. IMO it should be checking for proper device format string \u003cowner\u003e:\u003cresource\u003e.","commit_id":"c8a992abcc01dced6563018647d956c859382ac6"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"85b4fc6489aa8eeab2f7c68a816227d5c8f2de4c","unresolved":false,"context_lines":[{"line_number":848,"context_line":"            segmentations[segmentation_type].add(segmentation_id)"},{"line_number":849,"context_line":""},{"line_number":850,"context_line":""},{"line_number":851,"context_line":"def validate_device_owner(data, valid_values\u003dNone):"},{"line_number":852,"context_line":"    msg \u003d validate_string(data, max_len\u003dconstants.DEVICE_OWNER_MAX_LEN)"},{"line_number":853,"context_line":"    if data \u003d\u003d constants.DEVICE_OWNER_ROUTER_GW:"},{"line_number":854,"context_line":"        msg \u003d _(\"Ports of type: \u0027%s\u0027 cannot be created by via API\") % data"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a98d1f4_039e2d07","line":851,"range":{"start_line":851,"start_character":4,"end_line":851,"end_character":25},"updated":"2016-10-04 13:44:03.000000000","message":"The name is too generic. How about \"validate_device_owner_not_router_gw\".","commit_id":"c8a992abcc01dced6563018647d956c859382ac6"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"e044e742fd6494ee6bbc614d9459f5649a25be2f","unresolved":false,"context_lines":[{"line_number":848,"context_line":"            segmentations[segmentation_type].add(segmentation_id)"},{"line_number":849,"context_line":""},{"line_number":850,"context_line":""},{"line_number":851,"context_line":"def validate_device_owner(data, valid_values\u003dNone):"},{"line_number":852,"context_line":"    msg \u003d validate_string(data, max_len\u003dconstants.DEVICE_OWNER_MAX_LEN)"},{"line_number":853,"context_line":"    if data \u003d\u003d constants.DEVICE_OWNER_ROUTER_GW:"},{"line_number":854,"context_line":"        msg \u003d _(\"Ports of type: \u0027%s\u0027 cannot be created by via API\") % data"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a95cdbc_1ffae742","line":851,"range":{"start_line":851,"start_character":0,"end_line":851,"end_character":50},"in_reply_to":"3a98d1f4_e6922f4b","updated":"2016-10-05 03:50:27.000000000","message":"I have modified the body and the message, as I think such function can be used in the future as well by other operations.\nRequest you to please review the updated code, and thanks to both of you for your helpful comments","commit_id":"c8a992abcc01dced6563018647d956c859382ac6"},{"author":{"_account_id":5367,"name":"boden","email":"bodenvmw@gmail.com","username":"boden"},"change_message_id":"cc21fb7eae3f2e53420fc19b847f115f37ba8ea0","unresolved":false,"context_lines":[{"line_number":851,"context_line":"def validate_device_owner(data, valid_values\u003dNone):"},{"line_number":852,"context_line":"    msg \u003d validate_string(data, max_len\u003dconstants.DEVICE_OWNER_MAX_LEN)"},{"line_number":853,"context_line":"    if data \u003d\u003d constants.DEVICE_OWNER_ROUTER_GW:"},{"line_number":854,"context_line":"        msg \u003d _(\"Ports of type: \u0027%s\u0027 cannot be created by via API\") % data"},{"line_number":855,"context_line":"    return msg"},{"line_number":856,"context_line":""},{"line_number":857,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a98d1f4_06dc7b45","line":854,"range":{"start_line":854,"start_character":16,"end_line":854,"end_character":65},"updated":"2016-10-04 13:56:42.000000000","message":"A few comments:\n- This message is port specific, but the method name is generic. IMO either this should be generic, or the method name should more closely reflect intended usage.\n- Seems like this message should be more specific to the invalid data such as including info about the owner.","commit_id":"c8a992abcc01dced6563018647d956c859382ac6"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"e044e742fd6494ee6bbc614d9459f5649a25be2f","unresolved":false,"context_lines":[{"line_number":851,"context_line":"def validate_device_owner(data, valid_values\u003dNone):"},{"line_number":852,"context_line":"    msg \u003d validate_string(data, max_len\u003dconstants.DEVICE_OWNER_MAX_LEN)"},{"line_number":853,"context_line":"    if data \u003d\u003d constants.DEVICE_OWNER_ROUTER_GW:"},{"line_number":854,"context_line":"        msg \u003d _(\"Ports of type: \u0027%s\u0027 cannot be created by via API\") % data"},{"line_number":855,"context_line":"    return msg"},{"line_number":856,"context_line":""},{"line_number":857,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"1a95cdbc_3f95e3e3","line":854,"range":{"start_line":854,"start_character":16,"end_line":854,"end_character":65},"in_reply_to":"3a98d1f4_06dc7b45","updated":"2016-10-05 03:50:27.000000000","message":"I have updated the message, can you please review the updated code?","commit_id":"c8a992abcc01dced6563018647d956c859382ac6"},{"author":{"_account_id":14611,"name":"dasm","display_name":"dasm","email":"dsmigiel@redhat.com","username":"dasm"},"change_message_id":"8cd15de1691957b081c2eee540dab80bbe11b978","unresolved":false,"context_lines":[{"line_number":857,"context_line":"    Otherwise a human readable message is returned indicating why the"},{"line_number":858,"context_line":"    data is invalid."},{"line_number":859,"context_line":"    \"\"\""},{"line_number":860,"context_line":"    msg \u003d validate_string(data, max_len\u003dconstants.DEVICE_OWNER_MAX_LEN)"},{"line_number":861,"context_line":"    if valid_values:"},{"line_number":862,"context_line":"        if data \u003d\u003d valid_values:"},{"line_number":863,"context_line":"            msg \u003d _(\"%s is an invalid type of device_owner \""}],"source_content_type":"text/x-python","patch_set":4,"id":"fa6399be_301455f4","line":860,"updated":"2016-10-11 02:59:36.000000000","message":"This one should be moved below `if valid_values` check. It will save extra validation, if you\u0027ll have valid \u003d\u003d data.","commit_id":"5c4f49f4caac8e1e81c75118ad283ad192b115c2"},{"author":{"_account_id":5367,"name":"boden","email":"bodenvmw@gmail.com","username":"boden"},"change_message_id":"4b89e941fc74e7289ca137feef1a059d68f408b7","unresolved":false,"context_lines":[{"line_number":859,"context_line":"    \"\"\""},{"line_number":860,"context_line":"    msg \u003d validate_string(data, max_len\u003dconstants.DEVICE_OWNER_MAX_LEN)"},{"line_number":861,"context_line":"    if valid_values:"},{"line_number":862,"context_line":"        if data \u003d\u003d valid_values:"},{"line_number":863,"context_line":"            msg \u003d _(\"%s is an invalid type of device_owner \""},{"line_number":864,"context_line":"                    \"for this operation\") % data"},{"line_number":865,"context_line":"    return msg"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a95cdbc_a6ac6336","line":862,"range":{"start_line":862,"start_character":8,"end_line":862,"end_character":31},"updated":"2016-10-05 20:37:06.000000000","message":"What I was inferring is that IF valid_values is given the data should be validated using/calling validate_values() like I ref\u0027d in the last patch comment. They way its written here doesn\u0027t follow suit with how valid_values works (its supposed to be a collection of values that data can take on in some form).\n\n\nI\u0027m not overly familiar with how these device owners are validated, but my initial impression of how this validation might be handled:\n- If valid_values is given, assume its a collection of valid device owners (e.g. nova, neutron, etc.). In which case we can verify data is of the form \u003cdev_owner\u003e:\u003cresource\u003e and that dev_owner is in valid_values.\n- If valid_values is not given, we should validate the data is a string and is of the form \u003cdev_owner\u003e:\u003cresource\u003e or whatever is appropriate. That is, we verify the data does have the device owner format.\n\n\nThoughts?","commit_id":"5c4f49f4caac8e1e81c75118ad283ad192b115c2"},{"author":{"_account_id":14611,"name":"dasm","display_name":"dasm","email":"dsmigiel@redhat.com","username":"dasm"},"change_message_id":"8cd15de1691957b081c2eee540dab80bbe11b978","unresolved":false,"context_lines":[{"line_number":858,"context_line":"    data is invalid."},{"line_number":859,"context_line":"    \"\"\""},{"line_number":860,"context_line":"    msg \u003d validate_string(data, max_len\u003dconstants.DEVICE_OWNER_MAX_LEN)"},{"line_number":861,"context_line":"    if valid_values:"},{"line_number":862,"context_line":"        if data \u003d\u003d valid_values:"},{"line_number":863,"context_line":"            msg \u003d _(\"%s is an invalid type of device_owner \""},{"line_number":864,"context_line":"                    \"for this operation\") % data"},{"line_number":865,"context_line":"    return msg"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa6399be_70488d2e","line":862,"range":{"start_line":861,"start_character":0,"end_line":862,"end_character":32},"updated":"2016-10-11 02:59:36.000000000","message":"You can combine both these lines into one:\n\n if valid values and data \u003d\u003d valid_values:","commit_id":"5c4f49f4caac8e1e81c75118ad283ad192b115c2"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"88f66387cd3ab269695cc6d4bc03f2693445111e","unresolved":false,"context_lines":[{"line_number":859,"context_line":"    \"\"\""},{"line_number":860,"context_line":"    msg \u003d validate_string(data, max_len\u003dconstants.DEVICE_OWNER_MAX_LEN)"},{"line_number":861,"context_line":"    if valid_values:"},{"line_number":862,"context_line":"        if data \u003d\u003d valid_values:"},{"line_number":863,"context_line":"            msg \u003d _(\"%s is an invalid type of device_owner \""},{"line_number":864,"context_line":"                    \"for this operation\") % data"},{"line_number":865,"context_line":"    return msg"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa6399be_9e0e9d39","line":862,"range":{"start_line":862,"start_character":8,"end_line":862,"end_character":31},"in_reply_to":"1a95cdbc_a6ac6336","updated":"2016-10-13 10:35:36.000000000","message":"I have modified the code, can you have a look at the same , and let me know if it works?","commit_id":"5c4f49f4caac8e1e81c75118ad283ad192b115c2"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"e5842a4b2c0a8206052d50c63290ab69f1279ce0","unresolved":false,"context_lines":[{"line_number":859,"context_line":"    \"\"\""},{"line_number":860,"context_line":"    msg \u003d validate_string(data, max_len\u003dconstants.DEVICE_OWNER_MAX_LEN)"},{"line_number":861,"context_line":"    if valid_values:"},{"line_number":862,"context_line":"        if data \u003d\u003d valid_values:"},{"line_number":863,"context_line":"            msg \u003d _(\"%s is an invalid type of device_owner \""},{"line_number":864,"context_line":"                    \"for this operation\") % data"},{"line_number":865,"context_line":"    return msg"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a95cdbc_3aca1b6f","line":862,"range":{"start_line":862,"start_character":8,"end_line":862,"end_character":31},"in_reply_to":"1a95cdbc_a6ac6336","updated":"2016-10-06 04:08:09.000000000","message":"In this patch, we are verifying both the Device Owner and the resource.\nI think I need to rename it to validate_device to make sure that the input data is a combo of valid device_owners and resource, or in the form of dev_owner:resource","commit_id":"5c4f49f4caac8e1e81c75118ad283ad192b115c2"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"f6349580ad06b19abd0b709547d9335093b8edec","unresolved":false,"context_lines":[{"line_number":899,"context_line":"              \u0027type:boolean\u0027: validate_boolean,"},{"line_number":900,"context_line":"              \u0027type:integer\u0027: validate_integer,"},{"line_number":901,"context_line":"              \u0027type:list_of_unique_strings\u0027: validate_list_of_unique_strings,"},{"line_number":902,"context_line":"              \u0027type:device\u0027: validate_device_owner}"},{"line_number":903,"context_line":""},{"line_number":904,"context_line":""},{"line_number":905,"context_line":"def _to_validation_type(validation_type):"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa6399be_7912c30a","line":902,"updated":"2016-10-13 10:41:50.000000000","message":"Function\u0027s name \"validate_device_owner\" doesn\u0027t match with function definition in L851","commit_id":"2e3346501b23c93fd15da81b03c1d791781b216f"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"ee6dee0dac82e27a3fe62034b0f957660cefb8c6","unresolved":false,"context_lines":[{"line_number":899,"context_line":"              \u0027type:boolean\u0027: validate_boolean,"},{"line_number":900,"context_line":"              \u0027type:integer\u0027: validate_integer,"},{"line_number":901,"context_line":"              \u0027type:list_of_unique_strings\u0027: validate_list_of_unique_strings,"},{"line_number":902,"context_line":"              \u0027type:device\u0027: validate_device_owner}"},{"line_number":903,"context_line":""},{"line_number":904,"context_line":""},{"line_number":905,"context_line":"def _to_validation_type(validation_type):"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa6399be_39676b4e","line":902,"in_reply_to":"fa6399be_7912c30a","updated":"2016-10-13 10:47:27.000000000","message":"Oops","commit_id":"2e3346501b23c93fd15da81b03c1d791781b216f"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"89d18b0ee8dcd8e330e5b1897e35be179549a10a","unresolved":false,"context_lines":[{"line_number":859,"context_line":"    \"\"\""},{"line_number":860,"context_line":"    if valid_values:"},{"line_number":861,"context_line":"        msg \u003d validate_string(data, max_len\u003dconstants.DEVICE_OWNER_MAX_LEN)"},{"line_number":862,"context_line":"        if data \u003d\u003d valid_values:"},{"line_number":863,"context_line":"            msg \u003d validate_values(data\u003ddata, valid_values\u003dvalid_values)"},{"line_number":864,"context_line":"    return msg"},{"line_number":865,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"fa6399be_6424d65f","line":862,"range":{"start_line":862,"start_character":8,"end_line":862,"end_character":32},"updated":"2016-10-13 10:58:21.000000000","message":"i\u0027m not sure what you intended here.","commit_id":"c7dc8788abb7f830ebbf79fb753726dee468f2c2"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"32f3421456623f054bed00e83814b3601345d3ad","unresolved":false,"context_lines":[{"line_number":859,"context_line":"    \"\"\""},{"line_number":860,"context_line":"    if valid_values:"},{"line_number":861,"context_line":"        msg \u003d validate_string(data, max_len\u003dconstants.DEVICE_OWNER_MAX_LEN)"},{"line_number":862,"context_line":"        if data \u003d\u003d valid_values:"},{"line_number":863,"context_line":"            msg \u003d validate_values(data\u003ddata, valid_values\u003dvalid_values)"},{"line_number":864,"context_line":"    return msg"},{"line_number":865,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"fa6399be_587af6bd","line":862,"range":{"start_line":862,"start_character":8,"end_line":862,"end_character":32},"in_reply_to":"fa6399be_6424d65f","updated":"2016-10-17 02:33:53.000000000","message":"Done","commit_id":"c7dc8788abb7f830ebbf79fb753726dee468f2c2"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"89d18b0ee8dcd8e330e5b1897e35be179549a10a","unresolved":false,"context_lines":[{"line_number":860,"context_line":"    if valid_values:"},{"line_number":861,"context_line":"        msg \u003d validate_string(data, max_len\u003dconstants.DEVICE_OWNER_MAX_LEN)"},{"line_number":862,"context_line":"        if data \u003d\u003d valid_values:"},{"line_number":863,"context_line":"            msg \u003d validate_values(data\u003ddata, valid_values\u003dvalid_values)"},{"line_number":864,"context_line":"    return msg"},{"line_number":865,"context_line":""},{"line_number":866,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"fa6399be_84b72af4","line":863,"range":{"start_line":863,"start_character":18,"end_line":863,"end_character":33},"updated":"2016-10-13 10:58:21.000000000","message":"if you ensures the value matches to one of the given set anyway, what\u0027s the point of the max len check above?","commit_id":"c7dc8788abb7f830ebbf79fb753726dee468f2c2"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"32f3421456623f054bed00e83814b3601345d3ad","unresolved":false,"context_lines":[{"line_number":860,"context_line":"    if valid_values:"},{"line_number":861,"context_line":"        msg \u003d validate_string(data, max_len\u003dconstants.DEVICE_OWNER_MAX_LEN)"},{"line_number":862,"context_line":"        if data \u003d\u003d valid_values:"},{"line_number":863,"context_line":"            msg \u003d validate_values(data\u003ddata, valid_values\u003dvalid_values)"},{"line_number":864,"context_line":"    return msg"},{"line_number":865,"context_line":""},{"line_number":866,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"fa6399be_7875fac8","line":863,"range":{"start_line":863,"start_character":18,"end_line":863,"end_character":33},"in_reply_to":"fa6399be_84b72af4","updated":"2016-10-17 02:33:53.000000000","message":"Done","commit_id":"c7dc8788abb7f830ebbf79fb753726dee468f2c2"},{"author":{"_account_id":5367,"name":"boden","email":"bodenvmw@gmail.com","username":"boden"},"change_message_id":"75e535ad01b84e1b32c35db36b4f143a0e6c7b22","unresolved":false,"context_lines":[{"line_number":848,"context_line":"            segmentations[segmentation_type].add(segmentation_id)"},{"line_number":849,"context_line":""},{"line_number":850,"context_line":""},{"line_number":851,"context_line":"def validate_device(data, valid_values\u003dNone):"},{"line_number":852,"context_line":"    \"\"\"Validate if data is among a list of valid device owners."},{"line_number":853,"context_line":""},{"line_number":854,"context_line":"    :param data: The data to validate."}],"source_content_type":"text/x-python","patch_set":8,"id":"fa6399be_12c76796","line":851,"range":{"start_line":851,"start_character":4,"end_line":851,"end_character":19},"updated":"2016-10-17 13:03:44.000000000","message":"If validate_device() is just calling validate_values() I don\u0027t see what value it\u0027s providing here as consumers can just call validate_values() directly.\n\nIMO if this is going to validate devices, it should be validating the format, owner, etc. as I commented earlier.","commit_id":"44b186842195167e62aea36d1ad402d1d66fa8f7"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"8e04a050992872651a7b2ed90a026ebe41bbcac3","unresolved":false,"context_lines":[{"line_number":848,"context_line":"            segmentations[segmentation_type].add(segmentation_id)"},{"line_number":849,"context_line":""},{"line_number":850,"context_line":""},{"line_number":851,"context_line":"def validate_device(data, valid_values\u003dNone):"},{"line_number":852,"context_line":"    \"\"\"Validate if data is among a list of valid device owners."},{"line_number":853,"context_line":""},{"line_number":854,"context_line":"    :param data: The data to validate."}],"source_content_type":"text/x-python","patch_set":8,"id":"fa6399be_7eff2d01","line":851,"range":{"start_line":851,"start_character":4,"end_line":851,"end_character":19},"in_reply_to":"fa6399be_12c76796","updated":"2016-10-19 12:57:29.000000000","message":"I agree. In this form it is quite pointless. Based on the bug report, this should be named \u0027validate_device_owner_is_router_gw\u0027 and you can call validate_values with [DEVICE_OWNER_ROUTER_GW].","commit_id":"44b186842195167e62aea36d1ad402d1d66fa8f7"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"8e04a050992872651a7b2ed90a026ebe41bbcac3","unresolved":false,"context_lines":[{"line_number":897,"context_line":"              \u0027type:boolean\u0027: validate_boolean,"},{"line_number":898,"context_line":"              \u0027type:integer\u0027: validate_integer,"},{"line_number":899,"context_line":"              \u0027type:list_of_unique_strings\u0027: validate_list_of_unique_strings,"},{"line_number":900,"context_line":"              \u0027type:device\u0027: validate_device}"},{"line_number":901,"context_line":""},{"line_number":902,"context_line":""},{"line_number":903,"context_line":"def _to_validation_type(validation_type):"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa6399be_e1e0fcfc","line":900,"updated":"2016-10-19 12:57:29.000000000","message":"The names are too generic for the purpose outlined in the bug report. I would suggest:\n\n  \u0027type:router_gw_device\u0027:validate_device_owner_is_router_gw","commit_id":"44b186842195167e62aea36d1ad402d1d66fa8f7"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"33d921cfc2b455d86de23d605314804e3dc2b02f","unresolved":false,"context_lines":[{"line_number":858,"context_line":"    data is invalid."},{"line_number":859,"context_line":"    \"\"\""},{"line_number":860,"context_line":"    if valid_values:"},{"line_number":861,"context_line":"        msg \u003d validate_values(data\u003ddata,"},{"line_number":862,"context_line":"                              valid_values\u003d[constants.DEVICE_OWNER_ROUTER_GW])"},{"line_number":863,"context_line":"        return msg"},{"line_number":864,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"ba5da102_f8c75461","line":861,"updated":"2016-11-03 12:07:45.000000000","message":"is there any reason why we should not use validate_values? not sure why we need a new method here?","commit_id":"c219a08d360ecce0bb901579fd1959f15ae40f8b"},{"author":{"_account_id":5367,"name":"boden","email":"bodenvmw@gmail.com","username":"boden"},"change_message_id":"8f27f68ec6a723f86d0a7a9d874861e73b2bc991","unresolved":false,"context_lines":[{"line_number":858,"context_line":"    data is invalid."},{"line_number":859,"context_line":"    \"\"\""},{"line_number":860,"context_line":"    if valid_values:"},{"line_number":861,"context_line":"        msg \u003d validate_values(data\u003ddata,"},{"line_number":862,"context_line":"                              valid_values\u003d[constants.DEVICE_OWNER_ROUTER_GW])"},{"line_number":863,"context_line":"        return msg"},{"line_number":864,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"ba5da102_440bcb2b","line":861,"in_reply_to":"ba5da102_026487b6","updated":"2016-11-04 13:55:56.000000000","message":"From my POV its not about perfection, its about developing useful, generic APIs for multiple projects to consume. While I agree on our focus, I don\u0027t think it should proceed our ability to deliver quality function.","commit_id":"c219a08d360ecce0bb901579fd1959f15ae40f8b"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"4cae14aed711da4b47d0491fab8ab65e917a53de","unresolved":false,"context_lines":[{"line_number":858,"context_line":"    data is invalid."},{"line_number":859,"context_line":"    \"\"\""},{"line_number":860,"context_line":"    if valid_values:"},{"line_number":861,"context_line":"        msg \u003d validate_values(data\u003ddata,"},{"line_number":862,"context_line":"                              valid_values\u003d[constants.DEVICE_OWNER_ROUTER_GW])"},{"line_number":863,"context_line":"        return msg"},{"line_number":864,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"ba5da102_e01f4bb5","line":861,"in_reply_to":"ba5da102_2969398c","updated":"2016-11-06 14:24:31.000000000","message":"I have modified the code, need your inputs :)","commit_id":"c219a08d360ecce0bb901579fd1959f15ae40f8b"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"c098f55a37aee90daca5439d9ad0284831948633","unresolved":false,"context_lines":[{"line_number":858,"context_line":"    data is invalid."},{"line_number":859,"context_line":"    \"\"\""},{"line_number":860,"context_line":"    if valid_values:"},{"line_number":861,"context_line":"        msg \u003d validate_values(data\u003ddata,"},{"line_number":862,"context_line":"                              valid_values\u003d[constants.DEVICE_OWNER_ROUTER_GW])"},{"line_number":863,"context_line":"        return msg"},{"line_number":864,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"ba5da102_2969398c","line":861,"in_reply_to":"ba5da102_440bcb2b","updated":"2016-11-05 13:49:25.000000000","message":"I think Boden\u0027s point of covering a larger set of issues sounds right.\nLemme give it a shot in the next iteration","commit_id":"c219a08d360ecce0bb901579fd1959f15ae40f8b"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"cf8d02941f3d559119af85a0dcac1af935e34988","unresolved":false,"context_lines":[{"line_number":858,"context_line":"    data is invalid."},{"line_number":859,"context_line":"    \"\"\""},{"line_number":860,"context_line":"    if valid_values:"},{"line_number":861,"context_line":"        msg \u003d validate_values(data\u003ddata,"},{"line_number":862,"context_line":"                              valid_values\u003d[constants.DEVICE_OWNER_ROUTER_GW])"},{"line_number":863,"context_line":"        return msg"},{"line_number":864,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"ba5da102_026487b6","line":861,"in_reply_to":"ba5da102_534e573b","updated":"2016-11-03 17:15:35.000000000","message":"We all love to tweak things to perfection, but I feel I should re-iterate the Ocata goals for neutron-lib: Let\u0027s focus our efforts on rehoming neutron core code to break the sub-project dependencies.\n\nYes it\u0027s partly my fault for continuing to \"feed\" this review, sorry. I shall try to tear myself away for a while. ;)","commit_id":"c219a08d360ecce0bb901579fd1959f15ae40f8b"},{"author":{"_account_id":5367,"name":"boden","email":"bodenvmw@gmail.com","username":"boden"},"change_message_id":"11a4a9cc9cc33a2c5d11b0fc412d25b882a51dd3","unresolved":false,"context_lines":[{"line_number":858,"context_line":"    data is invalid."},{"line_number":859,"context_line":"    \"\"\""},{"line_number":860,"context_line":"    if valid_values:"},{"line_number":861,"context_line":"        msg \u003d validate_values(data\u003ddata,"},{"line_number":862,"context_line":"                              valid_values\u003d[constants.DEVICE_OWNER_ROUTER_GW])"},{"line_number":863,"context_line":"        return msg"},{"line_number":864,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"ba5da102_534e573b","line":861,"in_reply_to":"ba5da102_7bd417eb","updated":"2016-11-03 15:21:39.000000000","message":"If it were me, I\u0027d consider:\n- Create a new validate_device_owner_format(...) validator that ensures data is in the proper format (e.g. \u003cservice\u003e:\u003cresource\u003e). This one could accept valid_values as a list of valid device owner strings to constrain.\n- Create a new validate_device_owner_is_router_gw(data) validator that only accepts data (no valid_values) and just calls validate_device_owner_format(data, valid_values\u003d[constants.DEVICE_OWNER_ROUTER_GW])\n\n\nAlso: I\u0027m not following in this method why we only do validation if valid_values is passed, but then we just ignore it. As I suggested above, maybe this shouldn\u0027t even accept the valid_values kwarg.\n\n\nYou may also want to checkout some of the other patches in this space [2] and perhaps in particular [1].\n\n\n\n[1] https://review.openstack.org/#/q/project:openstack/neutron-lib+message:%22Verify+valid_values%22\n[2] https://review.openstack.org/#/c/390398/","commit_id":"c219a08d360ecce0bb901579fd1959f15ae40f8b"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"810c506e3210152d2874fce5ec461da3ce58586d","unresolved":false,"context_lines":[{"line_number":858,"context_line":"    data is invalid."},{"line_number":859,"context_line":"    \"\"\""},{"line_number":860,"context_line":"    if valid_values:"},{"line_number":861,"context_line":"        msg \u003d validate_values(data\u003ddata,"},{"line_number":862,"context_line":"                              valid_values\u003d[constants.DEVICE_OWNER_ROUTER_GW])"},{"line_number":863,"context_line":"        return msg"},{"line_number":864,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"ba5da102_7bd417eb","line":861,"in_reply_to":"ba5da102_f8c75461","updated":"2016-11-03 14:32:18.000000000","message":"Well, we are adding a new validator so we need a new method. \n\nThere are two options here.\n\n1. Use functools.partial(validate_values, ...)\n\nThis is short and simple, but the human readable message will be sub-optimal because we have only one valid value: \"\u003cdata\u003e is not in [network:router_gateway]\".\n\n2. Create a custom validation and error message in this new validator. Something like \"Device owner \u003cdata\u003e not allowed, expected device owner \u0027network:router_gateway\u0027\".","commit_id":"c219a08d360ecce0bb901579fd1959f15ae40f8b"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"dd076b81d23b003be7a4f42c7232b615a8475971","unresolved":false,"context_lines":[{"line_number":93,"context_line":"            return msg"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"def validate_values(data, valid_values\u003dNone):"},{"line_number":97,"context_line":"    \"\"\"Validate a data value based on a list of valid values."},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"    :param data: The data value to validate."}],"source_content_type":"text/x-python","patch_set":13,"id":"ba5da102_66a7d44f","line":96,"updated":"2016-11-06 17:08:31.000000000","message":"This has changed, causing the error in the TC","commit_id":"e1e867824944b63b0aa9fc0389467f8df958a2e1"},{"author":{"_account_id":5367,"name":"boden","email":"bodenvmw@gmail.com","username":"boden"},"change_message_id":"a41441b3a29bee912bd6d19d880c40b44412cf06","unresolved":false,"context_lines":[{"line_number":890,"context_line":"    data is invalid."},{"line_number":891,"context_line":"    \"\"\""},{"line_number":892,"context_line":"    msg \u003d None"},{"line_number":893,"context_line":"    r \u003d re.compile(\".*:.*\")"},{"line_number":894,"context_line":"    if not r.match(data):"},{"line_number":895,"context_line":"        msg \u003d (_(\"\u0027%(data)s\u0027 is not in the device owner format \""},{"line_number":896,"context_line":"                 \"\u003cservice\u003e:\u003cresource\u003e\") % {\"data\": data})"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a629dbe_4fb0488c","line":893,"range":{"start_line":893,"start_character":20,"end_line":893,"end_character":25},"updated":"2016-11-07 13:38:00.000000000","message":"This pattern allows formats like::\n\n    a:b:c:d\n\n\nIs that what we want? Seems we need to tighten this regex to ensure proper format.","commit_id":"adf768f5f9d2efd8d9cf4f987f6215045f2e2cf6"},{"author":{"_account_id":5367,"name":"boden","email":"bodenvmw@gmail.com","username":"boden"},"change_message_id":"a41441b3a29bee912bd6d19d880c40b44412cf06","unresolved":false,"context_lines":[{"line_number":895,"context_line":"        msg \u003d (_(\"\u0027%(data)s\u0027 is not in the device owner format \""},{"line_number":896,"context_line":"                 \"\u003cservice\u003e:\u003cresource\u003e\") % {\"data\": data})"},{"line_number":897,"context_line":"    elif valid_values:"},{"line_number":898,"context_line":"        msg \u003d validate_values(data\u003ddata,"},{"line_number":899,"context_line":"                              valid_values\u003dvalid_values,"},{"line_number":900,"context_line":"                              valid_values_display\u003dvalid_values)"},{"line_number":901,"context_line":"    if msg:"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a629dbe_8fe500aa","line":898,"range":{"start_line":898,"start_character":30,"end_line":898,"end_character":39},"updated":"2016-11-07 13:38:00.000000000","message":"Nit: no need to pass data as kwarg.","commit_id":"adf768f5f9d2efd8d9cf4f987f6215045f2e2cf6"},{"author":{"_account_id":5367,"name":"boden","email":"bodenvmw@gmail.com","username":"boden"},"change_message_id":"a41441b3a29bee912bd6d19d880c40b44412cf06","unresolved":false,"context_lines":[{"line_number":912,"context_line":"    data is invalid."},{"line_number":913,"context_line":"    \"\"\""},{"line_number":914,"context_line":"    msg \u003d validate_device_owner_format("},{"line_number":915,"context_line":"        data\u003ddata, valid_values\u003d[constants.DEVICE_OWNER_ROUTER_GW])"},{"line_number":916,"context_line":"    return msg"},{"line_number":917,"context_line":""},{"line_number":918,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"9a629dbe_cf9458ea","line":915,"range":{"start_line":915,"start_character":8,"end_line":915,"end_character":17},"updated":"2016-11-07 13:38:00.000000000","message":"Nit: no need to pass data as kwarg.","commit_id":"adf768f5f9d2efd8d9cf4f987f6215045f2e2cf6"},{"author":{"_account_id":5367,"name":"boden","email":"bodenvmw@gmail.com","username":"boden"},"change_message_id":"dc08c010394daa35e437b4d7ea2c504be3605273","unresolved":false,"context_lines":[{"line_number":890,"context_line":"    data is invalid."},{"line_number":891,"context_line":"    \"\"\""},{"line_number":892,"context_line":"    msg \u003d None"},{"line_number":893,"context_line":"    r \u003d re.compile(\"^[a-zA-Z]*:[a-zA-Z_]*$\")"},{"line_number":894,"context_line":"    if not r.match(data):"},{"line_number":895,"context_line":"        msg \u003d (_(\"\u0027%(data)s\u0027 is not in the device owner format \""},{"line_number":896,"context_line":"                 \"\u003cservice\u003e:\u003cresource\u003e\") % {\"data\": data})"}],"source_content_type":"text/x-python","patch_set":16,"id":"9a629dbe_30cec7b4","line":893,"range":{"start_line":893,"start_character":20,"end_line":893,"end_character":43},"updated":"2016-11-08 21:09:55.000000000","message":"Just to make sure, based on this pattern a format without a resource is valid? \ne.g.::\n\n    \u003cservice\u003e:\n\n\nThat\u0027s what you intend correct?","commit_id":"5ed038bbb35f1991df9b69e81c1fda68ee29bd7b"},{"author":{"_account_id":5367,"name":"boden","email":"bodenvmw@gmail.com","username":"boden"},"change_message_id":"1e4e4db4b4c657803e81bb5905e02546f1e69cfd","unresolved":false,"context_lines":[{"line_number":890,"context_line":"    data is invalid."},{"line_number":891,"context_line":"    \"\"\""},{"line_number":892,"context_line":"    msg \u003d None"},{"line_number":893,"context_line":"    r \u003d re.compile(\"^[a-zA-Z]*:[a-zA-Z_]*$\")"},{"line_number":894,"context_line":"    if not r.match(data):"},{"line_number":895,"context_line":"        msg \u003d (_(\"\u0027%(data)s\u0027 is not in the device owner format \""},{"line_number":896,"context_line":"                 \"\u003cservice\u003e:\u003cresource\u003e\") % {\"data\": data})"}],"source_content_type":"text/x-python","patch_set":16,"id":"7a77a97e_e53d4ebc","line":893,"range":{"start_line":893,"start_character":20,"end_line":893,"end_character":43},"in_reply_to":"7a77a97e_68607236","updated":"2016-11-16 20:22:47.000000000","message":"I haven\u0027t investigated the consumption of this to answer that question. If consumers permit a missing \u003cresource\u003e, then yes.","commit_id":"5ed038bbb35f1991df9b69e81c1fda68ee29bd7b"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"25efb9d484f91dc4ceb0c535012c1aba3c5a5d8a","unresolved":false,"context_lines":[{"line_number":890,"context_line":"    data is invalid."},{"line_number":891,"context_line":"    \"\"\""},{"line_number":892,"context_line":"    msg \u003d None"},{"line_number":893,"context_line":"    r \u003d re.compile(\"^[a-zA-Z]*:[a-zA-Z_]*$\")"},{"line_number":894,"context_line":"    if not r.match(data):"},{"line_number":895,"context_line":"        msg \u003d (_(\"\u0027%(data)s\u0027 is not in the device owner format \""},{"line_number":896,"context_line":"                 \"\u003cservice\u003e:\u003cresource\u003e\") % {\"data\": data})"}],"source_content_type":"text/x-python","patch_set":16,"id":"7a77a97e_90ebe977","line":893,"range":{"start_line":893,"start_character":20,"end_line":893,"end_character":43},"in_reply_to":"7a77a97e_e53d4ebc","updated":"2016-11-17 06:45:32.000000000","message":"Sorry, I should have answered No. maybe I was confused.\nI dont think [resource] would be optional for a device owner","commit_id":"5ed038bbb35f1991df9b69e81c1fda68ee29bd7b"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"207f8cf3b66b76f013172568e24581ef43fa4e35","unresolved":false,"context_lines":[{"line_number":890,"context_line":"    data is invalid."},{"line_number":891,"context_line":"    \"\"\""},{"line_number":892,"context_line":"    msg \u003d None"},{"line_number":893,"context_line":"    r \u003d re.compile(\"^[a-zA-Z]*:[a-zA-Z_]*$\")"},{"line_number":894,"context_line":"    if not r.match(data):"},{"line_number":895,"context_line":"        msg \u003d (_(\"\u0027%(data)s\u0027 is not in the device owner format \""},{"line_number":896,"context_line":"                 \"\u003cservice\u003e:\u003cresource\u003e\") % {\"data\": data})"}],"source_content_type":"text/x-python","patch_set":16,"id":"7a77a97e_68607236","line":893,"range":{"start_line":893,"start_character":20,"end_line":893,"end_character":43},"in_reply_to":"9a629dbe_30cec7b4","updated":"2016-11-15 20:24:48.000000000","message":"Yes, shouldnt that be the case?","commit_id":"5ed038bbb35f1991df9b69e81c1fda68ee29bd7b"},{"author":{"_account_id":5367,"name":"boden","email":"bodenvmw@gmail.com","username":"boden"},"change_message_id":"dc08c010394daa35e437b4d7ea2c504be3605273","unresolved":false,"context_lines":[{"line_number":894,"context_line":"    if not r.match(data):"},{"line_number":895,"context_line":"        msg \u003d (_(\"\u0027%(data)s\u0027 is not in the device owner format \""},{"line_number":896,"context_line":"                 \"\u003cservice\u003e:\u003cresource\u003e\") % {\"data\": data})"},{"line_number":897,"context_line":"    elif valid_values:"},{"line_number":898,"context_line":"        msg \u003d validate_values(data,"},{"line_number":899,"context_line":"                              valid_values\u003dvalid_values,"},{"line_number":900,"context_line":"                              valid_values_display\u003dvalid_values)"}],"source_content_type":"text/x-python","patch_set":16,"id":"9a629dbe_f008cfdb","line":897,"range":{"start_line":897,"start_character":4,"end_line":897,"end_character":22},"updated":"2016-11-08 21:09:55.000000000","message":"I\u0027d opt to change this to::\n\n    if not msg and valid_values:\n\nThat way we first validate the format, then if valid_values we also check its containment and the consumer is indicated in that order (via msg).","commit_id":"5ed038bbb35f1991df9b69e81c1fda68ee29bd7b"},{"author":{"_account_id":5367,"name":"boden","email":"bodenvmw@gmail.com","username":"boden"},"change_message_id":"1e4e4db4b4c657803e81bb5905e02546f1e69cfd","unresolved":false,"context_lines":[{"line_number":894,"context_line":"    if not r.match(data):"},{"line_number":895,"context_line":"        msg \u003d (_(\"\u0027%(data)s\u0027 is not in the device owner format \""},{"line_number":896,"context_line":"                 \"\u003cservice\u003e:\u003cresource\u003e\") % {\"data\": data})"},{"line_number":897,"context_line":"    elif valid_values:"},{"line_number":898,"context_line":"        msg \u003d validate_values(data,"},{"line_number":899,"context_line":"                              valid_values\u003dvalid_values,"},{"line_number":900,"context_line":"                              valid_values_display\u003dvalid_values)"}],"source_content_type":"text/x-python","patch_set":16,"id":"7a77a97e_c53bf2cf","line":897,"range":{"start_line":897,"start_character":4,"end_line":897,"end_character":22},"in_reply_to":"7a77a97e_48a81620","updated":"2016-11-16 20:22:47.000000000","message":"Clearly I\u0027m loosing my mind.","commit_id":"5ed038bbb35f1991df9b69e81c1fda68ee29bd7b"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"207f8cf3b66b76f013172568e24581ef43fa4e35","unresolved":false,"context_lines":[{"line_number":894,"context_line":"    if not r.match(data):"},{"line_number":895,"context_line":"        msg \u003d (_(\"\u0027%(data)s\u0027 is not in the device owner format \""},{"line_number":896,"context_line":"                 \"\u003cservice\u003e:\u003cresource\u003e\") % {\"data\": data})"},{"line_number":897,"context_line":"    elif valid_values:"},{"line_number":898,"context_line":"        msg \u003d validate_values(data,"},{"line_number":899,"context_line":"                              valid_values\u003dvalid_values,"},{"line_number":900,"context_line":"                              valid_values_display\u003dvalid_values)"}],"source_content_type":"text/x-python","patch_set":16,"id":"7a77a97e_48a81620","line":897,"range":{"start_line":897,"start_character":4,"end_line":897,"end_character":22},"in_reply_to":"9a629dbe_f008cfdb","updated":"2016-11-15 20:24:48.000000000","message":"I think the elsif will do that work , isnt it?","commit_id":"5ed038bbb35f1991df9b69e81c1fda68ee29bd7b"},{"author":{"_account_id":5367,"name":"boden","email":"bodenvmw@gmail.com","username":"boden"},"change_message_id":"523e8d7b9800cb5a8a98699ab39b53eb8905d931","unresolved":false,"context_lines":[{"line_number":890,"context_line":"    data is invalid."},{"line_number":891,"context_line":"    \"\"\""},{"line_number":892,"context_line":"    msg \u003d None"},{"line_number":893,"context_line":"    r \u003d re.compile(\"^[a-zA-Z]*:[a-zA-Z_]*$\")"},{"line_number":894,"context_line":"    if not r.match(data):"},{"line_number":895,"context_line":"        msg \u003d (_(\"\u0027%(data)s\u0027 is not in the device owner format \""},{"line_number":896,"context_line":"                 \"\u003cservice\u003e:\u003cresource\u003e\") % {\"data\": data})"}],"source_content_type":"text/x-python","patch_set":19,"id":"7a77a97e_1ecee32d","line":893,"range":{"start_line":893,"start_character":19,"end_line":893,"end_character":43},"updated":"2016-11-17 21:17:02.000000000","message":"I\u0027m still confused based on our chat back in patch 16. Best I can tell, the RE as-is accepts the following formats that I\u0027d expect it to not accept:\n\n  \u0027:\u0027\n  \u0027a:\u0027\n  \u0027:b\u0027","commit_id":"239e1c40c186f5054d92199f75a1bce671b5457d"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"fac053114e8548955245d8b6f865c2f79f7beba0","unresolved":false,"context_lines":[{"line_number":890,"context_line":"    data is invalid."},{"line_number":891,"context_line":"    \"\"\""},{"line_number":892,"context_line":"    msg \u003d None"},{"line_number":893,"context_line":"    r \u003d re.compile(\"^[a-zA-Z]*:[a-zA-Z_]*$\")"},{"line_number":894,"context_line":"    if not r.match(data):"},{"line_number":895,"context_line":"        msg \u003d (_(\"\u0027%(data)s\u0027 is not in the device owner format \""},{"line_number":896,"context_line":"                 \"\u003cservice\u003e:\u003cresource\u003e\") % {\"data\": data})"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a74a57a_4fa7084f","line":893,"range":{"start_line":893,"start_character":19,"end_line":893,"end_character":43},"in_reply_to":"7a77a97e_1ecee32d","updated":"2016-11-25 06:17:19.000000000","message":"Yureka ...I got it!!!","commit_id":"239e1c40c186f5054d92199f75a1bce671b5457d"},{"author":{"_account_id":5367,"name":"boden","email":"bodenvmw@gmail.com","username":"boden"},"change_message_id":"0ce3faa516b9bc9b6c983ab452d3d3b05ea5f653","unresolved":false,"context_lines":[{"line_number":881,"context_line":""},{"line_number":882,"context_line":"def validate_device_owner_format(data, valid_values\u003dNone):"},{"line_number":883,"context_line":"    \"\"\"Validate if data is among a list of valid device owners."},{"line_number":884,"context_line":""},{"line_number":885,"context_line":"    :param data: The data to validate."},{"line_number":886,"context_line":"    :param valid_values: List of Device Owners to be validated with."},{"line_number":887,"context_line":"    :returns: None if the device is in the proper device owner format"}],"source_content_type":"text/x-python","patch_set":20,"id":"5a74a57a_00b4a37d","line":884,"updated":"2016-11-28 18:48:42.000000000","message":"Nit: IMO a little confusing that we call this \u0027device owners\u0027 but in the message below we call it \u0027service\u0027 (\u003cservice\u003e:\u003cresource\u003e). It might be nice to clarify here with something like::\n\n   Valid device owner formatted strings are of the form:\n     \u003cservice\u003e:\u003cresource\u003e\n   Here the service is referred to as the device owner of the said resource.\n\n\nOr just change the format msg string to something like::\n\n   \u003cdevice_owner\u003e:\u003cdevice\u003e","commit_id":"5488eba1191eb095f222ea745394f9adb970ea1a"},{"author":{"_account_id":5367,"name":"boden","email":"bodenvmw@gmail.com","username":"boden"},"change_message_id":"0ce3faa516b9bc9b6c983ab452d3d3b05ea5f653","unresolved":false,"context_lines":[{"line_number":883,"context_line":"    \"\"\"Validate if data is among a list of valid device owners."},{"line_number":884,"context_line":""},{"line_number":885,"context_line":"    :param data: The data to validate."},{"line_number":886,"context_line":"    :param valid_values: List of Device Owners to be validated with."},{"line_number":887,"context_line":"    :returns: None if the device is in the proper device owner format"},{"line_number":888,"context_line":"    and exists in valid_values if valid_values is not None."},{"line_number":889,"context_line":"    Otherwise a human readable message is returned indicating why the"}],"source_content_type":"text/x-python","patch_set":20,"id":"5a74a57a_e0ea4773","line":886,"range":{"start_line":886,"start_character":25,"end_line":886,"end_character":68},"updated":"2016-11-28 18:48:42.000000000","message":"Nit: Something like: \"An optional collection of device owners to constrain the service value in the format\".","commit_id":"5488eba1191eb095f222ea745394f9adb970ea1a"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"e3ad267bbeb6a72e9304a6fa4ee6817f9aaf2065","unresolved":false,"context_lines":[{"line_number":897,"context_line":"    elif valid_values:"},{"line_number":898,"context_line":"        msg \u003d validate_values(data,"},{"line_number":899,"context_line":"                              valid_values\u003dvalid_values,"},{"line_number":900,"context_line":"                              valid_values_display\u003dvalid_values)"},{"line_number":901,"context_line":"    if msg:"},{"line_number":902,"context_line":"        LOG.debug(msg)"},{"line_number":903,"context_line":"    return msg"}],"source_content_type":"text/x-python","patch_set":20,"id":"5a74a57a_1b7b3629","line":900,"range":{"start_line":900,"start_character":30,"end_line":900,"end_character":63},"updated":"2016-11-28 18:59:46.000000000","message":"This is risky IMO. I would prefer validate_device_owner_is_router_gw() to create the display string and this function to pass it through.","commit_id":"5488eba1191eb095f222ea745394f9adb970ea1a"},{"author":{"_account_id":5367,"name":"boden","email":"bodenvmw@gmail.com","username":"boden"},"change_message_id":"0ce3faa516b9bc9b6c983ab452d3d3b05ea5f653","unresolved":false,"context_lines":[{"line_number":907,"context_line":"    \"\"\"Validate if data is a router gateway."},{"line_number":908,"context_line":""},{"line_number":909,"context_line":"    :param data: The data to validate."},{"line_number":910,"context_line":"    :returns: None if the device is Gateway Router."},{"line_number":911,"context_line":"    Otherwise a human readable message is returned indicating why the"},{"line_number":912,"context_line":"    data is invalid."},{"line_number":913,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":20,"id":"5a74a57a_c0e62be7","line":910,"range":{"start_line":910,"start_character":26,"end_line":910,"end_character":32},"updated":"2016-11-28 18:48:42.000000000","message":"Nit: \u0027device owner\u0027","commit_id":"5488eba1191eb095f222ea745394f9adb970ea1a"},{"author":{"_account_id":5367,"name":"boden","email":"bodenvmw@gmail.com","username":"boden"},"change_message_id":"0ce3faa516b9bc9b6c983ab452d3d3b05ea5f653","unresolved":false,"context_lines":[{"line_number":907,"context_line":"    \"\"\"Validate if data is a router gateway."},{"line_number":908,"context_line":""},{"line_number":909,"context_line":"    :param data: The data to validate."},{"line_number":910,"context_line":"    :returns: None if the device is Gateway Router."},{"line_number":911,"context_line":"    Otherwise a human readable message is returned indicating why the"},{"line_number":912,"context_line":"    data is invalid."},{"line_number":913,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":20,"id":"5a74a57a_a0184fd4","line":910,"range":{"start_line":910,"start_character":36,"end_line":910,"end_character":50},"updated":"2016-11-28 18:48:42.000000000","message":"Nit: IMHO \u0027Gateway Router\u0027 is a bit misleading. I\u0027d consider just changing this whole sentence to something like::\n\n   \"None if the data is in device owner format and the device owner is constants.DEVICE_OWNER_ROUTER_GW.\"","commit_id":"5488eba1191eb095f222ea745394f9adb970ea1a"},{"author":{"_account_id":5367,"name":"boden","email":"bodenvmw@gmail.com","username":"boden"},"change_message_id":"0ce3faa516b9bc9b6c983ab452d3d3b05ea5f653","unresolved":false,"context_lines":[{"line_number":911,"context_line":"    Otherwise a human readable message is returned indicating why the"},{"line_number":912,"context_line":"    data is invalid."},{"line_number":913,"context_line":"    \"\"\""},{"line_number":914,"context_line":"    msg \u003d validate_device_owner_format("},{"line_number":915,"context_line":"        data, valid_values\u003d[constants.DEVICE_OWNER_ROUTER_GW])"},{"line_number":916,"context_line":"    return msg"},{"line_number":917,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"5a74a57a_a0bccfbb","line":914,"updated":"2016-11-28 18:48:42.000000000","message":"Nit: I don\u0027t see a reason to even define \u0027msg\u0027... I\u0027d just replace it with::\n\n   return validate_device_owner_format(...)","commit_id":"5488eba1191eb095f222ea745394f9adb970ea1a"},{"author":{"_account_id":8726,"name":"Victor Morales","email":"chipahuac@hotmail.com","username":"electrocucaracha"},"change_message_id":"fda062912f32a82824e7b4bf6874e5052882e8f0","unresolved":false,"context_lines":[{"line_number":886,"context_line":"    :param data: The data to validate."},{"line_number":887,"context_line":"    :param valid_values: Optional list of Device Owners to be validated with."},{"line_number":888,"context_line":"    :returns: None if the device is in the proper device owner format"},{"line_number":889,"context_line":"    and exists in valid_values if valid_values is not None."},{"line_number":890,"context_line":"    Otherwise a human readable message is returned indicating why the"},{"line_number":891,"context_line":"    data is invalid."},{"line_number":892,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":25,"id":"5a74a57a_95a3179e","line":889,"range":{"start_line":889,"start_character":4,"end_line":889,"end_character":7},"updated":"2016-11-30 22:17:27.000000000","message":"nit: isn\u0027t an \"or\" here? in the other case the elif needs to be change it for an if sentence.","commit_id":"0d214f6a7bd8eedfa21c5b3839c5f5997ec75188"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"cb3676bafcb4647a635d1fdc7b4977eb83e5382c","unresolved":false,"context_lines":[{"line_number":894,"context_line":"    r \u003d re.compile(\"^[a-zA-Z]+:[a-zA-Z_]+$\")"},{"line_number":895,"context_line":"    if not r.match(data):"},{"line_number":896,"context_line":"        msg \u003d (_(\"\u0027%(data)s\u0027 is not in the device owner format \""},{"line_number":897,"context_line":"                 \"\u003cdevice_owner\u003e:\u003cdevice\u003e\") % {\"data\": data})"},{"line_number":898,"context_line":"    elif valid_values:"},{"line_number":899,"context_line":"        msg \u003d validate_values(data,"},{"line_number":900,"context_line":"                              valid_values\u003dvalid_values,"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a74a57a_6b1db765","line":897,"updated":"2016-11-30 23:18:07.000000000","message":"when did we ever decide that this is the format a device owner must be? This seems too restrictive.","commit_id":"0d214f6a7bd8eedfa21c5b3839c5f5997ec75188"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"87601d6c3d685b9c495bc5e2939fec785ad58672","unresolved":false,"context_lines":[{"line_number":894,"context_line":"    r \u003d re.compile(\"^[a-zA-Z]+:[a-zA-Z_]+$\")"},{"line_number":895,"context_line":"    if not r.match(data):"},{"line_number":896,"context_line":"        msg \u003d (_(\"\u0027%(data)s\u0027 is not in the device owner format \""},{"line_number":897,"context_line":"                 \"\u003cdevice_owner\u003e:\u003cdevice\u003e\") % {\"data\": data})"},{"line_number":898,"context_line":"    elif valid_values:"},{"line_number":899,"context_line":"        msg \u003d validate_values(data,"},{"line_number":900,"context_line":"                              valid_values\u003dvalid_values,"}],"source_content_type":"text/x-python","patch_set":25,"id":"3a71b18c_7126a1bb","line":897,"in_reply_to":"3a71b18c_4459e859","updated":"2016-12-02 03:09:05.000000000","message":"We never have had any issues. No user should ever fiddle with the device_owner themselves, the fact that this attribute is writable is an accident because of nova needing to update it when booting VMs off ports.","commit_id":"0d214f6a7bd8eedfa21c5b3839c5f5997ec75188"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"fd5f974f474c5ac22d179fb933b6f04ed6d88977","unresolved":false,"context_lines":[{"line_number":894,"context_line":"    r \u003d re.compile(\"^[a-zA-Z]+:[a-zA-Z_]+$\")"},{"line_number":895,"context_line":"    if not r.match(data):"},{"line_number":896,"context_line":"        msg \u003d (_(\"\u0027%(data)s\u0027 is not in the device owner format \""},{"line_number":897,"context_line":"                 \"\u003cdevice_owner\u003e:\u003cdevice\u003e\") % {\"data\": data})"},{"line_number":898,"context_line":"    elif valid_values:"},{"line_number":899,"context_line":"        msg \u003d validate_values(data,"},{"line_number":900,"context_line":"                              valid_values\u003dvalid_values,"}],"source_content_type":"text/x-python","patch_set":25,"id":"3a71b18c_670503c1","line":897,"in_reply_to":"3a71b18c_7126a1bb","updated":"2016-12-02 04:17:10.000000000","message":"Hmm, ok.","commit_id":"0d214f6a7bd8eedfa21c5b3839c5f5997ec75188"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"a1ca772eabf29678e3e92022932976cdb15f6fc6","unresolved":false,"context_lines":[{"line_number":894,"context_line":"    r \u003d re.compile(\"^[a-zA-Z]+:[a-zA-Z_]+$\")"},{"line_number":895,"context_line":"    if not r.match(data):"},{"line_number":896,"context_line":"        msg \u003d (_(\"\u0027%(data)s\u0027 is not in the device owner format \""},{"line_number":897,"context_line":"                 \"\u003cdevice_owner\u003e:\u003cdevice\u003e\") % {\"data\": data})"},{"line_number":898,"context_line":"    elif valid_values:"},{"line_number":899,"context_line":"        msg \u003d validate_values(data,"},{"line_number":900,"context_line":"                              valid_values\u003dvalid_values,"}],"source_content_type":"text/x-python","patch_set":25,"id":"3a71b18c_4459e859","line":897,"in_reply_to":"3a71b18c_840ea732","updated":"2016-12-02 02:39:33.000000000","message":"Shouldnt Neutron team define a rule for this ?\nI mean, if it is free form , then we can have lots of issues later down the line when new devices are to be put up.\nI know this is not mentioned anywhere but I also feel that the devices should have a nomenclature as Device:XYZ....\n\nThe start can definitely be with Device, but the second part ( XYZ ) can be anything. This is although a separate topic, but nonetheless something important which may impact us later.","commit_id":"0d214f6a7bd8eedfa21c5b3839c5f5997ec75188"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"d3170bb055406773fef5381b26eab93d85bc9e4b","unresolved":false,"context_lines":[{"line_number":894,"context_line":"    r \u003d re.compile(\"^[a-zA-Z]+:[a-zA-Z_]+$\")"},{"line_number":895,"context_line":"    if not r.match(data):"},{"line_number":896,"context_line":"        msg \u003d (_(\"\u0027%(data)s\u0027 is not in the device owner format \""},{"line_number":897,"context_line":"                 \"\u003cdevice_owner\u003e:\u003cdevice\u003e\") % {\"data\": data})"},{"line_number":898,"context_line":"    elif valid_values:"},{"line_number":899,"context_line":"        msg \u003d validate_values(data,"},{"line_number":900,"context_line":"                              valid_values\u003dvalid_values,"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a74a57a_eaeb7810","line":897,"in_reply_to":"5a74a57a_6b1db765","updated":"2016-12-01 05:46:51.000000000","message":"@Armando : is there any document which specifies the nomenclature for device owners ?\nIt would be of great help","commit_id":"0d214f6a7bd8eedfa21c5b3839c5f5997ec75188"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"f3641980e82d5aae2cc23ee3d9527cd315aac42f","unresolved":false,"context_lines":[{"line_number":894,"context_line":"    r \u003d re.compile(\"^[a-zA-Z]+:[a-zA-Z_]+$\")"},{"line_number":895,"context_line":"    if not r.match(data):"},{"line_number":896,"context_line":"        msg \u003d (_(\"\u0027%(data)s\u0027 is not in the device owner format \""},{"line_number":897,"context_line":"                 \"\u003cdevice_owner\u003e:\u003cdevice\u003e\") % {\"data\": data})"},{"line_number":898,"context_line":"    elif valid_values:"},{"line_number":899,"context_line":"        msg \u003d validate_values(data,"},{"line_number":900,"context_line":"                              valid_values\u003dvalid_values,"}],"source_content_type":"text/x-python","patch_set":25,"id":"3a71b18c_840ea732","line":897,"in_reply_to":"5a74a57a_eaeb7810","updated":"2016-12-02 00:07:38.000000000","message":"no, that\u0027s the thing. This is today considered free form. There are well known constants, but there\u0027s no written rule on how these should conform.","commit_id":"0d214f6a7bd8eedfa21c5b3839c5f5997ec75188"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"1a6be3807e9b867e04fbb0006557d9bea390527f","unresolved":false,"context_lines":[{"line_number":908,"context_line":"    \"\"\"Validate if data is a router gateway."},{"line_number":909,"context_line":""},{"line_number":910,"context_line":"    :param data: The data to validate."},{"line_number":911,"context_line":"    :returns: None if the data is in device owner format and data is"},{"line_number":912,"context_line":"    constants.DEVICE_OWNER_ROUTER_GW. Otherwise a human readable message"},{"line_number":913,"context_line":"    is returned indicating why the data is invalid."},{"line_number":914,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":25,"id":"5a74a57a_3d12370a","line":911,"range":{"start_line":911,"start_character":22,"end_line":911,"end_character":60},"updated":"2016-11-30 16:59:56.000000000","message":"Nit: remove this part, it\u0027s redundant","commit_id":"0d214f6a7bd8eedfa21c5b3839c5f5997ec75188"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"1a6be3807e9b867e04fbb0006557d9bea390527f","unresolved":false,"context_lines":[{"line_number":913,"context_line":"    is returned indicating why the data is invalid."},{"line_number":914,"context_line":"    \"\"\""},{"line_number":915,"context_line":"    value \u003d [constants.DEVICE_OWNER_ROUTER_GW]"},{"line_number":916,"context_line":"    return validate_device_owner_format("},{"line_number":917,"context_line":"        data, valid_values\u003dvalue,"},{"line_number":918,"context_line":"        valid_values_display\u003dvalue)"},{"line_number":919,"context_line":""}],"source_content_type":"text/x-python","patch_set":25,"id":"5a74a57a_9df18bd0","line":916,"updated":"2016-11-30 16:59:56.000000000","message":"Nit: can you line the args up like lines 899-901 so the formatting is consistent?","commit_id":"0d214f6a7bd8eedfa21c5b3839c5f5997ec75188"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"1a6be3807e9b867e04fbb0006557d9bea390527f","unresolved":false,"context_lines":[{"line_number":915,"context_line":"    value \u003d [constants.DEVICE_OWNER_ROUTER_GW]"},{"line_number":916,"context_line":"    return validate_device_owner_format("},{"line_number":917,"context_line":"        data, valid_values\u003dvalue,"},{"line_number":918,"context_line":"        valid_values_display\u003dvalue)"},{"line_number":919,"context_line":""},{"line_number":920,"context_line":""},{"line_number":921,"context_line":"# Dictionary that maintains a list of validation functions"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a74a57a_1d265b68","line":918,"range":{"start_line":918,"start_character":29,"end_line":918,"end_character":34},"updated":"2016-11-30 16:59:56.000000000","message":"Super nit: to clarify intent you could write \u0027str(value)\u0027 here.","commit_id":"0d214f6a7bd8eedfa21c5b3839c5f5997ec75188"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"cb3676bafcb4647a635d1fdc7b4977eb83e5382c","unresolved":false,"context_lines":[{"line_number":953,"context_line":"              \u0027type:boolean\u0027: validate_boolean,"},{"line_number":954,"context_line":"              \u0027type:integer\u0027: validate_integer,"},{"line_number":955,"context_line":"              \u0027type:list_of_unique_strings\u0027: validate_list_of_unique_strings,"},{"line_number":956,"context_line":"              \u0027type:device_router_gw\u0027: validate_device_owner_is_router_gw}"},{"line_number":957,"context_line":""},{"line_number":958,"context_line":""},{"line_number":959,"context_line":"def _to_validation_type(validation_type):"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a74a57a_eba46709","line":956,"updated":"2016-11-30 23:18:07.000000000","message":"I don\u0027t understand: where would you use such a validator? The device_owner attribute for a generic resource? Why would we even call it router_gw if the values can change?\n\nTo be honest I think device_owner of ports it\u0027s an abomination that has caused us so many troubles and it should be killed but we can\u0027t because we can\u0027t break the world. But going forward? I don\u0027t see the use case.","commit_id":"0d214f6a7bd8eedfa21c5b3839c5f5997ec75188"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"87601d6c3d685b9c495bc5e2939fec785ad58672","unresolved":false,"context_lines":[{"line_number":953,"context_line":"              \u0027type:boolean\u0027: validate_boolean,"},{"line_number":954,"context_line":"              \u0027type:integer\u0027: validate_integer,"},{"line_number":955,"context_line":"              \u0027type:list_of_unique_strings\u0027: validate_list_of_unique_strings,"},{"line_number":956,"context_line":"              \u0027type:device_router_gw\u0027: validate_device_owner_is_router_gw}"},{"line_number":957,"context_line":""},{"line_number":958,"context_line":""},{"line_number":959,"context_line":"def _to_validation_type(validation_type):"}],"source_content_type":"text/x-python","patch_set":25,"id":"3a71b18c_f141d13a","line":956,"in_reply_to":"3a71b18c_64042417","updated":"2016-12-02 03:09:05.000000000","message":"Personally, I have not seen this being a burning issue, and lacking a real use case for this validator, I don\u0027t see the point in adding it to the library.","commit_id":"0d214f6a7bd8eedfa21c5b3839c5f5997ec75188"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"a1ca772eabf29678e3e92022932976cdb15f6fc6","unresolved":false,"context_lines":[{"line_number":953,"context_line":"              \u0027type:boolean\u0027: validate_boolean,"},{"line_number":954,"context_line":"              \u0027type:integer\u0027: validate_integer,"},{"line_number":955,"context_line":"              \u0027type:list_of_unique_strings\u0027: validate_list_of_unique_strings,"},{"line_number":956,"context_line":"              \u0027type:device_router_gw\u0027: validate_device_owner_is_router_gw}"},{"line_number":957,"context_line":""},{"line_number":958,"context_line":""},{"line_number":959,"context_line":"def _to_validation_type(validation_type):"}],"source_content_type":"text/x-python","patch_set":25,"id":"3a71b18c_64042417","line":956,"in_reply_to":"3a71b18c_6420ab9c","updated":"2016-12-02 02:39:33.000000000","message":"Ok, I understand that.\nI guess this bug can be resolved without adding a validator and instead adding a check when we are binding the port to the router, if the port is already a router:gateway type.\n\nBut I would still like your , Henry\u0027s ,boden\u0027s and other reviewer\u0027s opinion if we should define a nomenclature for the various devices  we have in neutron.","commit_id":"0d214f6a7bd8eedfa21c5b3839c5f5997ec75188"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"fd5f974f474c5ac22d179fb933b6f04ed6d88977","unresolved":false,"context_lines":[{"line_number":953,"context_line":"              \u0027type:boolean\u0027: validate_boolean,"},{"line_number":954,"context_line":"              \u0027type:integer\u0027: validate_integer,"},{"line_number":955,"context_line":"              \u0027type:list_of_unique_strings\u0027: validate_list_of_unique_strings,"},{"line_number":956,"context_line":"              \u0027type:device_router_gw\u0027: validate_device_owner_is_router_gw}"},{"line_number":957,"context_line":""},{"line_number":958,"context_line":""},{"line_number":959,"context_line":"def _to_validation_type(validation_type):"}],"source_content_type":"text/x-python","patch_set":25,"id":"3a71b18c_4701e7ad","line":956,"in_reply_to":"3a71b18c_f141d13a","updated":"2016-12-02 04:17:10.000000000","message":"I agree that the validator need not be added here.\nThis sort of check can be moved to the point where the port is actually being bounded to the router.\n\nMy point was on the other point, that if Neutron should define a rule for device owners.\nYou have already mentioned your idea for the same","commit_id":"0d214f6a7bd8eedfa21c5b3839c5f5997ec75188"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"f3641980e82d5aae2cc23ee3d9527cd315aac42f","unresolved":false,"context_lines":[{"line_number":953,"context_line":"              \u0027type:boolean\u0027: validate_boolean,"},{"line_number":954,"context_line":"              \u0027type:integer\u0027: validate_integer,"},{"line_number":955,"context_line":"              \u0027type:list_of_unique_strings\u0027: validate_list_of_unique_strings,"},{"line_number":956,"context_line":"              \u0027type:device_router_gw\u0027: validate_device_owner_is_router_gw}"},{"line_number":957,"context_line":""},{"line_number":958,"context_line":""},{"line_number":959,"context_line":"def _to_validation_type(validation_type):"}],"source_content_type":"text/x-python","patch_set":25,"id":"3a71b18c_6420ab9c","line":956,"in_reply_to":"5a74a57a_ea39b8a4","updated":"2016-12-02 00:07:38.000000000","message":"The validator as being added here won\u0027t solve the issue you are describing. This is an input validator, behind called in [1]. If the input value in the resource field violates the validation, an invalid error is returned. There\u0027s no knowledge of the current state of the resource associated to the field and nor there should be.\n\n[1] http://git.openstack.org/cgit/openstack/neutron/tree/neutron/api/v2/attributes.py#n332","commit_id":"0d214f6a7bd8eedfa21c5b3839c5f5997ec75188"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"d3170bb055406773fef5381b26eab93d85bc9e4b","unresolved":false,"context_lines":[{"line_number":953,"context_line":"              \u0027type:boolean\u0027: validate_boolean,"},{"line_number":954,"context_line":"              \u0027type:integer\u0027: validate_integer,"},{"line_number":955,"context_line":"              \u0027type:list_of_unique_strings\u0027: validate_list_of_unique_strings,"},{"line_number":956,"context_line":"              \u0027type:device_router_gw\u0027: validate_device_owner_is_router_gw}"},{"line_number":957,"context_line":""},{"line_number":958,"context_line":""},{"line_number":959,"context_line":"def _to_validation_type(validation_type):"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a74a57a_ea39b8a4","line":956,"in_reply_to":"5a74a57a_eba46709","updated":"2016-12-01 05:46:51.000000000","message":"Well, the current issue wanted to avoid assigning a port \"X\" to a router \"A\" when the port \"X\" is already acting as a router_gateway port for router \"B\".\n\nWhile this validator can work here, but what I think you are stating is NOT to put validation here, but , if we need to resolve this bug, then perform such checks at a different code section.\n\nI would like your inputs on this.","commit_id":"0d214f6a7bd8eedfa21c5b3839c5f5997ec75188"}],"neutron_lib/tests/unit/api/test_validators.py":[{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"89d18b0ee8dcd8e330e5b1897e35be179549a10a","unresolved":false,"context_lines":[{"line_number":986,"context_line":""},{"line_number":987,"context_line":"    def test_validate_device_owner_invalid_device(self):"},{"line_number":988,"context_line":"        device \u003d constants.DEVICE_OWNER_ROUTER_GW"},{"line_number":989,"context_line":"        msg \u003d validators.validate_device_owner("},{"line_number":990,"context_line":"            device, constants.DEVICE_OWNER_ROUTER_GW)"},{"line_number":991,"context_line":"        self.assertEqual("},{"line_number":992,"context_line":"            \"%s is an invalid type of device_owner \""}],"source_content_type":"text/x-python","patch_set":7,"id":"fa6399be_84448a1b","line":989,"range":{"start_line":989,"start_character":25,"end_line":989,"end_character":46},"updated":"2016-10-13 10:58:21.000000000","message":"method name mismatch?","commit_id":"c7dc8788abb7f830ebbf79fb753726dee468f2c2"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"32f3421456623f054bed00e83814b3601345d3ad","unresolved":false,"context_lines":[{"line_number":986,"context_line":""},{"line_number":987,"context_line":"    def test_validate_device_owner_invalid_device(self):"},{"line_number":988,"context_line":"        device \u003d constants.DEVICE_OWNER_ROUTER_GW"},{"line_number":989,"context_line":"        msg \u003d validators.validate_device_owner("},{"line_number":990,"context_line":"            device, constants.DEVICE_OWNER_ROUTER_GW)"},{"line_number":991,"context_line":"        self.assertEqual("},{"line_number":992,"context_line":"            \"%s is an invalid type of device_owner \""}],"source_content_type":"text/x-python","patch_set":7,"id":"fa6399be_386f02fd","line":989,"range":{"start_line":989,"start_character":25,"end_line":989,"end_character":46},"in_reply_to":"fa6399be_84448a1b","updated":"2016-10-17 02:33:53.000000000","message":"Done","commit_id":"c7dc8788abb7f830ebbf79fb753726dee468f2c2"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"d0dca855d1d01822cb5744787a568ceb10ea4815","unresolved":false,"context_lines":[{"line_number":986,"context_line":""},{"line_number":987,"context_line":"    def test_validate_device_owner_invalid_device(self):"},{"line_number":988,"context_line":"        device \u003d constants.DEVICE_OWNER_ROUTER_GW"},{"line_number":989,"context_line":"        msg \u003d validators.validate_device("},{"line_number":990,"context_line":"            device, [constants.DEVICE_OWNER_ROUTER_GW])"},{"line_number":991,"context_line":"        self.assertEqual(("},{"line_number":992,"context_line":"            \"\u0027%(data)s\u0027 is not in %(valid_values)s\") %"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa6399be_df30a558","line":989,"range":{"start_line":989,"start_character":8,"end_line":989,"end_character":11},"updated":"2016-10-17 08:34:20.000000000","message":"Do you want to use the variable in the assert text? If not, pep8 is complaining.","commit_id":"44b186842195167e62aea36d1ad402d1d66fa8f7"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"d0dca855d1d01822cb5744787a568ceb10ea4815","unresolved":false,"context_lines":[{"line_number":990,"context_line":"            device, [constants.DEVICE_OWNER_ROUTER_GW])"},{"line_number":991,"context_line":"        self.assertEqual(("},{"line_number":992,"context_line":"            \"\u0027%(data)s\u0027 is not in %(valid_values)s\") %"},{"line_number":993,"context_line":"               {\u0027data\u0027: data,"},{"line_number":994,"context_line":"                \u0027valid_values\u0027: [constants.DEVICE_OWNER_ROUTER_GW]})"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa6399be_5f3d7529","line":993,"range":{"start_line":993,"start_character":24,"end_line":993,"end_character":25},"updated":"2016-10-17 08:34:20.000000000","message":"Where is \u0027data\u0027 defined?","commit_id":"44b186842195167e62aea36d1ad402d1d66fa8f7"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"51b8b12a7b05ed2162f59f60d80d08b0c3e93f43","unresolved":false,"context_lines":[{"line_number":987,"context_line":"    def test_validate_device_owner_invalid_device(self):"},{"line_number":988,"context_line":"        device \u003d constants.DEVICE_OWNER_ROUTER_GW"},{"line_number":989,"context_line":"        msg \u003d validators.validate_device_owner_is_router_gw(device)"},{"line_number":990,"context_line":"        self.assertEqual((msg,str(device) + \" is not in \" +"},{"line_number":991,"context_line":"                          [constants.DEVICE_OWNER_ROUTER_GW])"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba5da102_3de0decc","line":990,"range":{"start_line":990,"start_character":25,"end_line":990,"end_character":26},"updated":"2016-11-03 11:54:08.000000000","message":"extraneous paren. Should have (expected, actual) for consistency.","commit_id":"c219a08d360ecce0bb901579fd1959f15ae40f8b"}],"releasenotes/notes/device_owner_validtor-407493a4535b24c6.yaml":[{"author":{"_account_id":5367,"name":"boden","email":"bodenvmw@gmail.com","username":"boden"},"change_message_id":"71cff30ac17785a03483b1b6852a89d9b5de8d64","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"other:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    A new validator has been added , which allows validation of the"},{"line_number":5,"context_line":"    device owner in the format `\u003cservice\u003e:\u003cresource\u003e`. If the device"},{"line_number":6,"context_line":"    owner is not of the specified format, error message is raised."}],"source_content_type":"text/x-yaml","patch_set":17,"id":"7a77a97e_45d1a256","line":4,"range":{"start_line":4,"start_character":34,"end_line":4,"end_character":35},"updated":"2016-11-16 20:26:47.000000000","message":"Nit: no space needed.","commit_id":"ededfd7e6f20094744f2c4832d54b040500da2d3"},{"author":{"_account_id":5367,"name":"boden","email":"bodenvmw@gmail.com","username":"boden"},"change_message_id":"71cff30ac17785a03483b1b6852a89d9b5de8d64","unresolved":false,"context_lines":[{"line_number":2,"context_line":"other:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    A new validator has been added , which allows validation of the"},{"line_number":5,"context_line":"    device owner in the format `\u003cservice\u003e:\u003cresource\u003e`. If the device"},{"line_number":6,"context_line":"    owner is not of the specified format, error message is raised."}],"source_content_type":"text/x-yaml","patch_set":17,"id":"7a77a97e_c5c5b28e","line":5,"range":{"start_line":5,"start_character":32,"end_line":5,"end_character":52},"updated":"2016-11-16 20:26:47.000000000","message":"So based on our discussion that the current validator allows a missing \u003cresource\u003e, should we note that here? e.g. to me the format is \u003cservice\u003e:[resource] where [resource] is optional.","commit_id":"ededfd7e6f20094744f2c4832d54b040500da2d3"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"ae52916d10d51074d6ae39718c2257717fda6952","unresolved":false,"context_lines":[{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    A new validator has been added, which allows validation of the"},{"line_number":5,"context_line":"    device owner in the format `\u003cservice\u003e:\u003cresource\u003e`. If the device"},{"line_number":6,"context_line":"    owner is not of the specified format, error message is raised."}],"source_content_type":"text/x-yaml","patch_set":18,"id":"7a77a97e_4396e93f","line":6,"updated":"2016-11-17 07:07:11.000000000","message":"i guess it\u0027s better to mention the name of the validator.","commit_id":"45ed93c57623c7849eda10c70b4b6c42e4f8759d"},{"author":{"_account_id":17776,"name":"Reedip","email":"reedip.banerjee@gmail.com","username":"Reedip"},"change_message_id":"c926bd878bc92e028475f7e1418d233ad665f160","unresolved":false,"context_lines":[{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    A new validator has been added, which allows validation of the"},{"line_number":5,"context_line":"    device owner in the format `\u003cservice\u003e:\u003cresource\u003e`. If the device"},{"line_number":6,"context_line":"    owner is not of the specified format, error message is raised."}],"source_content_type":"text/x-yaml","patch_set":18,"id":"7a77a97e_461fd76b","line":6,"in_reply_to":"7a77a97e_4396e93f","updated":"2016-11-17 07:57:00.000000000","message":"Done","commit_id":"45ed93c57623c7849eda10c70b4b6c42e4f8759d"},{"author":{"_account_id":5367,"name":"boden","email":"bodenvmw@gmail.com","username":"boden"},"change_message_id":"0ce3faa516b9bc9b6c983ab452d3d3b05ea5f653","unresolved":false,"context_lines":[{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    A new validator ``validate_device_owner_is_router_gw`` has been"},{"line_number":5,"context_line":"    added, which allows validation of the device owner in the format"},{"line_number":6,"context_line":"    `\u003cservice\u003e:\u003cresource\u003e`. If the device owner is not of the specified"},{"line_number":7,"context_line":"    format, error message is raised."}],"source_content_type":"text/x-yaml","patch_set":20,"id":"5a74a57a_0062e309","line":6,"range":{"start_line":6,"start_character":5,"end_line":6,"end_character":25},"updated":"2016-11-28 18:48:42.000000000","message":"Please see my nits in validators.py in regards to this format.","commit_id":"5488eba1191eb095f222ea745394f9adb970ea1a"},{"author":{"_account_id":5367,"name":"boden","email":"bodenvmw@gmail.com","username":"boden"},"change_message_id":"0ce3faa516b9bc9b6c983ab452d3d3b05ea5f653","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":20,"id":"5a74a57a_801b5380","line":8,"updated":"2016-11-28 18:48:42.000000000","message":"We also need to include something about validate_device_owner_format as this is a new public API as well.","commit_id":"5488eba1191eb095f222ea745394f9adb970ea1a"},{"author":{"_account_id":5367,"name":"boden","email":"bodenvmw@gmail.com","username":"boden"},"change_message_id":"0f3773646fa124d21f507ce1e686fec907177e38","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"other:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Two new validators ``validate_device_owner_is_router_gw`` and"},{"line_number":5,"context_line":"    ``validate_device_owner_format`` have been added."}],"source_content_type":"text/x-yaml","patch_set":24,"id":"5a74a57a_6f6b4d86","line":2,"range":{"start_line":2,"start_character":0,"end_line":2,"end_character":5},"updated":"2016-11-29 17:27:44.000000000","message":"Maybe this is a nit, but don\u0027t we want this under the \u0027features\u0027 section since these are in fact new API (features)?","commit_id":"cef72b5d97cfcd42f5c5a7f66b812825fadcddbf"}]}