)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":14131,"name":"shaohef","email":"shaohe.feng@intel.com","username":"shaohefeng"},"change_message_id":"ef1b502c84d08bd15f62bfb96f56616d0749be9c","unresolved":false,"context_lines":[{"line_number":13,"context_line":"1. the AllocationController accepts only the parsed infos, OS-ACC should"},{"line_number":14,"context_line":"parse the flavor.extra_specs into useful infos."},{"line_number":15,"context_line":"2. there should be a lock mechanism while one request comes in and trying to"},{"line_number":16,"context_line":"update the DB. @lockutils.synchronized() is used currently, it should be"},{"line_number":17,"context_line":"improved by add a lock mechanism in lower layer like DB layer."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"You can test this patch by using:"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3f79a3b5_55e62db2","line":16,"range":{"start_line":16,"start_character":15,"end_line":16,"end_character":40},"updated":"2018-08-24 11:14:12.000000000","message":"This can avoid race for a temporary solution. \n\nWe can improve it later.\n\nThanks.","commit_id":"2c1640dad061754f810be556b40b88cacbf514b2"},{"author":{"_account_id":27458,"name":"Li Liu","email":"liliueecg@gmail.com","username":"liliu"},"change_message_id":"7e3f995d4a03440bcd36dda65e24f21372c17098","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add Allocation/Deallocation API"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Currently Cyborg use patch method to alloc/dealloc a deployable. It should be"},{"line_number":10,"context_line":"a new API to handle that."},{"line_number":11,"context_line":"In this patch, 2 APIs(AllocationController,DeallocationController) are define."},{"line_number":12,"context_line":"FYI:"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3f79a3b5_d14778a2","line":9,"range":{"start_line":9,"start_character":17,"end_line":9,"end_character":20},"updated":"2018-08-28 04:06:39.000000000","message":"uses","commit_id":"8463a500da72c713a9e6ff146ac348c691186fbe"},{"author":{"_account_id":27458,"name":"Li Liu","email":"liliueecg@gmail.com","username":"liliu"},"change_message_id":"7e3f995d4a03440bcd36dda65e24f21372c17098","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Currently Cyborg use patch method to alloc/dealloc a deployable. It should be"},{"line_number":10,"context_line":"a new API to handle that."},{"line_number":11,"context_line":"In this patch, 2 APIs(AllocationController,DeallocationController) are define."},{"line_number":12,"context_line":"FYI:"},{"line_number":13,"context_line":"1. the AllocationController accepts only the parsed infos, OS-ACC should"},{"line_number":14,"context_line":"parse the flavor.extra_specs into useful infos."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3f79a3b5_71c8e420","line":11,"range":{"start_line":11,"start_character":71,"end_line":11,"end_character":77},"updated":"2018-08-28 04:06:39.000000000","message":"defined","commit_id":"8463a500da72c713a9e6ff146ac348c691186fbe"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"337f21e87aa4684f06135644ac075a3d3209563b","unresolved":false,"context_lines":[{"line_number":3,"context_line":"AuthorDate: 2018-08-22 10:33:00 +0800"},{"line_number":4,"context_line":"Commit:     Xinran WANG \u003cxin-ran.wang@intel.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2018-08-28 08:31:05 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add Allocation/Deallocation API"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Currently Cyborg uses patch method to alloc/dealloc a deployable. It should be"},{"line_number":10,"context_line":"a new API to handle that."},{"line_number":11,"context_line":"In this patch, 2 APIs(AllocationController,DeallocationController) are defined."},{"line_number":12,"context_line":"FYI:"},{"line_number":13,"context_line":"1. the AllocationController accepts only the parsed infos, OS-ACC should"},{"line_number":14,"context_line":"parse the flavor.extra_specs into useful infos."},{"line_number":15,"context_line":"2. there should be a lock mechanism while one request comes in and trying to"},{"line_number":16,"context_line":"update the DB. @lockutils.synchronized() is used currently, it should be"},{"line_number":17,"context_line":"improved by add a lock mechanism in lower layer like DB layer."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"You can test this patch by using:"},{"line_number":20,"context_line":"curl -i -X POST  http://localhost:6666/accelerators/deployables/allocations -H \"Content-Type: application/json\" -H \"Accept: application/json\" -H \"X-Auth-Token: $(openstack token issue -f value -c id)\" -d \u0027{\"pf\":[{\"vendor\": \"8086\", \"parent_uuid\": null, \"interface_type\": \"pci\"}],\"instance_uuid\": \"5c050a23-7f0e-4ec3-aeb2-f92cc3b7f1e0\", \"host\": \"host1\"}\u0027"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"and"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"curl -i -X POST  http://localhost:6666/accelerators/deployables/deallocations/{instance_uuid} -H \"Content-Type: application/json\" -H \"Accept: application/json\" -H \"X-Auth-Token: $(openstack token issue -f value -c id)\""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"Change-Id: Ib8d0f15e32010cc19cefab9ff9417ad9bda3542e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"3f79a3b5_59bb51c3","line":26,"range":{"start_line":6,"start_character":0,"end_line":26,"end_character":52},"updated":"2018-08-30 09:06:04.000000000","message":"it looks like the gate jobs for cyborge are not enforcing linght lenght limits. you should proably wrap this","commit_id":"30cf57faa060c5cdec0d501793eb98f0f0469192"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"73e9eaa101db455291d64c8c27bb1a5eb1da7913","unresolved":false,"context_lines":[{"line_number":20,"context_line":"curl -i -X POST \\"},{"line_number":21,"context_line":"http://localhost:6666/accelerators/deployables/actions/allocation \\"},{"line_number":22,"context_line":"H \"Content-Type: application/json\" -H \"Accept: application/json\" \\"},{"line_number":23,"context_line":"-H \"X-Auth-Token: $(openstack token issue -f value -c id)\" \\"},{"line_number":24,"context_line":"-d \u0027{\"pf\":[{\"vendor\": \"8086\", \"parent_uuid\": null, \"interface_type\": \"pci\"}], \\"},{"line_number":25,"context_line":"\"instance_uuid\": \"5c050a23-7f0e-4ec3-aeb2-f92cc3b7f1e0\", \"host\": \"host1\"}\u0027"},{"line_number":26,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"3f79a3b5_2100b000","line":23,"updated":"2018-09-24 22:45:22.000000000","message":"just incase you were not aware you can pull the token out into  a env variable and reuse it\nseveral times. if you are locally developing allocating a token every time is not terrible however be aware that on a large cloud this pattern is not advisable as each token issues is stored in the db forever if uuid tokens are used instead of ferrnet tokens which can lead to 100GBs of expired tokens on large clouds like cern.","commit_id":"9f90465df75f99e41e17b668fb61d604cb851182"},{"author":{"_account_id":25738,"name":"Xinran WANG","email":"xin-ran.wang@intel.com","username":"Xinran"},"change_message_id":"45726cf4274101032c594ebeea6922021e7be58c","unresolved":false,"context_lines":[{"line_number":20,"context_line":"curl -i -X POST \\"},{"line_number":21,"context_line":"http://localhost:6666/accelerators/deployables/actions/allocation \\"},{"line_number":22,"context_line":"H \"Content-Type: application/json\" -H \"Accept: application/json\" \\"},{"line_number":23,"context_line":"-H \"X-Auth-Token: $(openstack token issue -f value -c id)\" \\"},{"line_number":24,"context_line":"-d \u0027{\"pf\":[{\"vendor\": \"8086\", \"parent_uuid\": null, \"interface_type\": \"pci\"}], \\"},{"line_number":25,"context_line":"\"instance_uuid\": \"5c050a23-7f0e-4ec3-aeb2-f92cc3b7f1e0\", \"host\": \"host1\"}\u0027"},{"line_number":26,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"3f79a3b5_56f1cc95","line":23,"in_reply_to":"3f79a3b5_2100b000","updated":"2018-09-26 10:08:00.000000000","message":"thanks for you comments, I use this statement here because I want make the one who want test this patch can follow these instruction directly :)","commit_id":"9f90465df75f99e41e17b668fb61d604cb851182"}],"cyborg/api/controllers/v1/deployable/actions.py":[{"author":{"_account_id":14131,"name":"shaohef","email":"shaohe.feng@intel.com","username":"shaohefeng"},"change_message_id":"0339f989ae2f00338ce83fe1a9cb936efa64a8f7","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        super(DeallocationController, self).__init__(**kwargs)"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    @policy.authorize_wsgi(\"cyborg:deallocation\", \"deallocate\", False)"},{"line_number":70,"context_line":"    @lockutils.synchronized(\u0027deallocation\u0027, \u0027cyborg-\u0027)"},{"line_number":71,"context_line":"    @expose.expose(None, types.uuid, types.jsontype)"},{"line_number":72,"context_line":"    def post(self, instance_uuid\u003dNone, deployable_uuids\u003dNone):"},{"line_number":73,"context_line":"        obj_deps \u003d []"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_3a712077","line":70,"range":{"start_line":70,"start_character":4,"end_line":70,"end_character":54},"updated":"2018-08-24 09:47:26.000000000","message":"Deallocation may not need a lock","commit_id":"2c1640dad061754f810be556b40b88cacbf514b2"},{"author":{"_account_id":17813,"name":"wangzhh","email":"wzh_1993@126.com","username":"wangzhh"},"change_message_id":"03568f302eabc735df2d8b9f564956dfd602c1a2","unresolved":false,"context_lines":[{"line_number":25,"context_line":"from oslo_concurrency import lockutils"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"class AllocationController(rest.RestController):"},{"line_number":29,"context_line":"    \"\"\" REST controller for Deployables Allocation"},{"line_number":30,"context_line":"    \"\"\""},{"line_number":31,"context_line":"    def __init__(self, **kwargs):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_9bbb3d74","line":28,"updated":"2018-08-29 10:30:52.000000000","message":"xinran,\nCould u merge these two controllers? For example, ActionController with two methods.","commit_id":"8463a500da72c713a9e6ff146ac348c691186fbe"},{"author":{"_account_id":27458,"name":"Li Liu","email":"liliueecg@gmail.com","username":"liliu"},"change_message_id":"7e3f995d4a03440bcd36dda65e24f21372c17098","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        for device_type, specs in body.items():"},{"line_number":44,"context_line":"            for spec in specs:"},{"line_number":45,"context_line":"                res_num \u003d spec.get(\"num\", 1)"},{"line_number":46,"context_line":"                spec[\"type\"] \u003d device_type"},{"line_number":47,"context_line":"                spec[\"instance_uuid\"] \u003d None"},{"line_number":48,"context_line":"                spec[\"assignable\"] \u003d True"},{"line_number":49,"context_line":"                deps \u003d objects.Deployable.get_by_filter("}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_51ea8873","line":46,"range":{"start_line":46,"start_character":31,"end_line":46,"end_character":42},"updated":"2018-08-28 04:06:39.000000000","message":"why does the device_type can be changed during allocation?","commit_id":"8463a500da72c713a9e6ff146ac348c691186fbe"},{"author":{"_account_id":25738,"name":"Xinran WANG","email":"xin-ran.wang@intel.com","username":"Xinran"},"change_message_id":"4942d973a52bf94aaecd1d02c3a4c3fd3cd61a4d","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        for device_type, specs in body.items():"},{"line_number":44,"context_line":"            for spec in specs:"},{"line_number":45,"context_line":"                res_num \u003d spec.get(\"num\", 1)"},{"line_number":46,"context_line":"                spec[\"type\"] \u003d device_type"},{"line_number":47,"context_line":"                spec[\"instance_uuid\"] \u003d None"},{"line_number":48,"context_line":"                spec[\"assignable\"] \u003d True"},{"line_number":49,"context_line":"                deps \u003d objects.Deployable.get_by_filter("}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_ff46fa20","line":46,"range":{"start_line":46,"start_character":31,"end_line":46,"end_character":42},"in_reply_to":"3f79a3b5_51ea8873","updated":"2018-08-28 08:31:43.000000000","message":"this field will not be changed during allocation, this assignment aims at assigning device_type which is body.key() to spec like other body.values().","commit_id":"8463a500da72c713a9e6ff146ac348c691186fbe"},{"author":{"_account_id":27458,"name":"Li Liu","email":"liliueecg@gmail.com","username":"liliu"},"change_message_id":"7e3f995d4a03440bcd36dda65e24f21372c17098","unresolved":false,"context_lines":[{"line_number":45,"context_line":"                res_num \u003d spec.get(\"num\", 1)"},{"line_number":46,"context_line":"                spec[\"type\"] \u003d device_type"},{"line_number":47,"context_line":"                spec[\"instance_uuid\"] \u003d None"},{"line_number":48,"context_line":"                spec[\"assignable\"] \u003d True"},{"line_number":49,"context_line":"                deps \u003d objects.Deployable.get_by_filter("},{"line_number":50,"context_line":"                    pecan.request.context, filters\u003dspec)"},{"line_number":51,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_11009033","line":48,"range":{"start_line":48,"start_character":22,"end_line":48,"end_character":32},"updated":"2018-08-28 04:06:39.000000000","message":"Let\u0027s discuss the usage of this field in the upcoming meeting.","commit_id":"8463a500da72c713a9e6ff146ac348c691186fbe"},{"author":{"_account_id":28687,"name":"JiangGuocai","email":"jiang.guocai@zte.com.cn","username":"JiangGuocai"},"change_message_id":"747e604a7364ac4cb009fa1a562e43dc890baee0","unresolved":false,"context_lines":[{"line_number":61,"context_line":""},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"class DeallocationController(rest.RestController):"},{"line_number":64,"context_line":"    \"\"\" REST controller for Deployables Allocation"},{"line_number":65,"context_line":"    \"\"\""},{"line_number":66,"context_line":"    def __init__(self, **kwargs):"},{"line_number":67,"context_line":"        super(DeallocationController, self).__init__(**kwargs)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_5acc1c33","line":64,"range":{"start_line":64,"start_character":40,"end_line":64,"end_character":50},"updated":"2018-09-06 02:58:54.000000000","message":"Lost De?  Deallocation","commit_id":"30cf57faa060c5cdec0d501793eb98f0f0469192"},{"author":{"_account_id":17813,"name":"wangzhh","email":"wzh_1993@126.com","username":"wangzhh"},"change_message_id":"03568f302eabc735df2d8b9f564956dfd602c1a2","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        super(DeallocationController, self).__init__(**kwargs)"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    @policy.authorize_wsgi(\"cyborg:deallocation\", \"deallocate\", False)"},{"line_number":70,"context_line":"    @expose.expose(None, types.uuid, types.jsontype)"},{"line_number":71,"context_line":"    def post(self, instance_uuid\u003dNone, deployable_uuids\u003dNone):"},{"line_number":72,"context_line":"        obj_deps \u003d []"},{"line_number":73,"context_line":"        if not instance_uuid and not deployable_uuids:"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_1b22cdbb","line":70,"updated":"2018-08-29 10:30:52.000000000","message":"It\u0027s better to define a new type like DeployableAction.","commit_id":"30cf57faa060c5cdec0d501793eb98f0f0469192"},{"author":{"_account_id":17813,"name":"wangzhh","email":"wzh_1993@126.com","username":"wangzhh"},"change_message_id":"03568f302eabc735df2d8b9f564956dfd602c1a2","unresolved":false,"context_lines":[{"line_number":73,"context_line":"        if not instance_uuid and not deployable_uuids:"},{"line_number":74,"context_line":"            raise exception.BadRequest()"},{"line_number":75,"context_line":"        if deployable_uuids:"},{"line_number":76,"context_line":"            for k, uuid in deployable_uuids.items():"},{"line_number":77,"context_line":"                obj_dep \u003d objects.Deployable.get(pecan.request.context, uuid)"},{"line_number":78,"context_line":"                obj_deps.append(obj_dep)"},{"line_number":79,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_8d06a41c","line":76,"updated":"2018-08-29 10:30:52.000000000","message":"Do u use the \u0027k\u0027? If not,\nIt should be \nfor _, uuid in deployable_uuids.items():\n    ...","commit_id":"30cf57faa060c5cdec0d501793eb98f0f0469192"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"337f21e87aa4684f06135644ac075a3d3209563b","unresolved":false,"context_lines":[{"line_number":73,"context_line":"        if not instance_uuid and not deployable_uuids:"},{"line_number":74,"context_line":"            raise exception.BadRequest()"},{"line_number":75,"context_line":"        if deployable_uuids:"},{"line_number":76,"context_line":"            for k, uuid in deployable_uuids.items():"},{"line_number":77,"context_line":"                obj_dep \u003d objects.Deployable.get(pecan.request.context, uuid)"},{"line_number":78,"context_line":"                obj_deps.append(obj_dep)"},{"line_number":79,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_d9cbe147","line":76,"in_reply_to":"3f79a3b5_8d06a41c","updated":"2018-08-30 09:06:04.000000000","message":"if deployable_uuids is a dictionaty then you should just use deployable_uuids.values()","commit_id":"30cf57faa060c5cdec0d501793eb98f0f0469192"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"73e9eaa101db455291d64c8c27bb1a5eb1da7913","unresolved":false,"context_lines":[{"line_number":39,"context_line":"        super(DeployableActions, self).__init__(**kwargs)"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"    @policy.authorize_wsgi(\"cyborg:deployable:actions\", \"allocation\", False)"},{"line_number":42,"context_line":"    @lockutils.synchronized(\u0027allocation\u0027, \u0027cyborg-\u0027)"},{"line_number":43,"context_line":"    @expose.expose(DeployableCollection, body\u003dtypes.jsontype)"},{"line_number":44,"context_line":"    def allocation(self, body):"},{"line_number":45,"context_line":"        context \u003d pecan.request.context"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_a10b6020","line":42,"updated":"2018-09-24 22:45:22.000000000","message":"why are you locking at the api level?\nthe db layer should be able to enforce transtation sematics\nof a claim with out serialising alls api calls to allocate.","commit_id":"9f90465df75f99e41e17b668fb61d604cb851182"},{"author":{"_account_id":25738,"name":"Xinran WANG","email":"xin-ran.wang@intel.com","username":"Xinran"},"change_message_id":"45726cf4274101032c594ebeea6922021e7be58c","unresolved":false,"context_lines":[{"line_number":39,"context_line":"        super(DeployableActions, self).__init__(**kwargs)"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"    @policy.authorize_wsgi(\"cyborg:deployable:actions\", \"allocation\", False)"},{"line_number":42,"context_line":"    @lockutils.synchronized(\u0027allocation\u0027, \u0027cyborg-\u0027)"},{"line_number":43,"context_line":"    @expose.expose(DeployableCollection, body\u003dtypes.jsontype)"},{"line_number":44,"context_line":"    def allocation(self, body):"},{"line_number":45,"context_line":"        context \u003d pecan.request.context"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_164034a2","line":42,"in_reply_to":"3f79a3b5_a10b6020","updated":"2018-09-26 10:08:00.000000000","message":"Yes, I agree with you. I also plan to implement it at DB layer in the following patches.","commit_id":"9f90465df75f99e41e17b668fb61d604cb851182"},{"author":{"_account_id":23735,"name":"sumitjami","email":"sjamgade@suse.com","username":"nthforloop"},"change_message_id":"d561d74cf0b86a8c5cb521e4eb0b31120113d953","unresolved":false,"context_lines":[{"line_number":45,"context_line":"        context \u003d pecan.request.context"},{"line_number":46,"context_line":"        instance_uuid \u003d body.pop(\"instance_uuid\")"},{"line_number":47,"context_line":"        host \u003d body.pop(\"host\")"},{"line_number":48,"context_line":"        if not instance_uuid or not host:"},{"line_number":49,"context_line":"            return DeployableCollection.convert_with_links([])"},{"line_number":50,"context_line":"        obj_deps \u003d []"},{"line_number":51,"context_line":"        for device_type, specs in body.items():"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_e1fd866d","line":48,"updated":"2018-10-23 07:56:20.000000000","message":"somekind of logging or return with a warning would be good","commit_id":"9f90465df75f99e41e17b668fb61d604cb851182"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"73e9eaa101db455291d64c8c27bb1a5eb1da7913","unresolved":false,"context_lines":[{"line_number":55,"context_line":"                spec[\"instance_uuid\"] \u003d None"},{"line_number":56,"context_line":"                spec[\"assignable\"] \u003d True"},{"line_number":57,"context_line":"                reserve_opts \u003d {device_type: res_num}"},{"line_number":58,"context_line":"                reservations \u003d QUOTAS.reserve(context,"},{"line_number":59,"context_line":"                                              reserve_opts)"},{"line_number":60,"context_line":"                deps \u003d objects.Deployable.get_by_filter(context, filters\u003dspec)"},{"line_number":61,"context_line":"                if len(deps) \u003c res_num:"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_41184c75","line":58,"updated":"2018-09-24 22:45:22.000000000","message":"if you refactor the inner loop to only build a list of reservation opts you\nshould be able to prefrom the reservations, quota checks and conductor update outside of the loop.","commit_id":"9f90465df75f99e41e17b668fb61d604cb851182"},{"author":{"_account_id":25738,"name":"Xinran WANG","email":"xin-ran.wang@intel.com","username":"Xinran"},"change_message_id":"45726cf4274101032c594ebeea6922021e7be58c","unresolved":false,"context_lines":[{"line_number":55,"context_line":"                spec[\"instance_uuid\"] \u003d None"},{"line_number":56,"context_line":"                spec[\"assignable\"] \u003d True"},{"line_number":57,"context_line":"                reserve_opts \u003d {device_type: res_num}"},{"line_number":58,"context_line":"                reservations \u003d QUOTAS.reserve(context,"},{"line_number":59,"context_line":"                                              reserve_opts)"},{"line_number":60,"context_line":"                deps \u003d objects.Deployable.get_by_filter(context, filters\u003dspec)"},{"line_number":61,"context_line":"                if len(deps) \u003c res_num:"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_715652d1","line":58,"in_reply_to":"3f79a3b5_41184c75","updated":"2018-09-26 10:08:00.000000000","message":"I am not sure if I got your point. IMO, this loop is to implement batch allocation. For example, there maybe several accelerators request in specs. We should deal with it one by one, and quota reserve, and quota usage update should be done one by one.","commit_id":"9f90465df75f99e41e17b668fb61d604cb851182"},{"author":{"_account_id":27458,"name":"Li Liu","email":"liliueecg@gmail.com","username":"liliu"},"change_message_id":"0e66a5d0f6c43e496f7d2ea5ab8ad70a64393b44","unresolved":false,"context_lines":[{"line_number":55,"context_line":"                spec[\"instance_uuid\"] \u003d None"},{"line_number":56,"context_line":"                spec[\"assignable\"] \u003d True"},{"line_number":57,"context_line":"                reserve_opts \u003d {device_type: res_num}"},{"line_number":58,"context_line":"                reservations \u003d QUOTAS.reserve(context,"},{"line_number":59,"context_line":"                                              reserve_opts)"},{"line_number":60,"context_line":"                deps \u003d objects.Deployable.get_by_filter(context, filters\u003dspec)"},{"line_number":61,"context_line":"                if len(deps) \u003c res_num:"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_4d3f6fb4","line":58,"in_reply_to":"3f79a3b5_715652d1","updated":"2018-10-03 01:42:38.000000000","message":"Shall we provide a DB atomic operation here for batch updating?","commit_id":"9f90465df75f99e41e17b668fb61d604cb851182"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"73e9eaa101db455291d64c8c27bb1a5eb1da7913","unresolved":false,"context_lines":[{"line_number":66,"context_line":"                obj_deps \u003d obj_deps + deps[:res_num]"},{"line_number":67,"context_line":"        for obj in obj_deps:"},{"line_number":68,"context_line":"            obj[\"instance_uuid\"] \u003d instance_uuid"},{"line_number":69,"context_line":"            obj[\"assignable\"] \u003d False"},{"line_number":70,"context_line":"            pecan.request.conductor_api.deployable_update(context, obj)"},{"line_number":71,"context_line":"        return DeployableCollection.convert_with_links(obj_deps)"},{"line_number":72,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_6115483d","line":69,"updated":"2018-09-24 22:45:22.000000000","message":"this should likely be done as part of the reserve fucntion","commit_id":"9f90465df75f99e41e17b668fb61d604cb851182"},{"author":{"_account_id":25738,"name":"Xinran WANG","email":"xin-ran.wang@intel.com","username":"Xinran"},"change_message_id":"45726cf4274101032c594ebeea6922021e7be58c","unresolved":false,"context_lines":[{"line_number":66,"context_line":"                obj_deps \u003d obj_deps + deps[:res_num]"},{"line_number":67,"context_line":"        for obj in obj_deps:"},{"line_number":68,"context_line":"            obj[\"instance_uuid\"] \u003d instance_uuid"},{"line_number":69,"context_line":"            obj[\"assignable\"] \u003d False"},{"line_number":70,"context_line":"            pecan.request.conductor_api.deployable_update(context, obj)"},{"line_number":71,"context_line":"        return DeployableCollection.convert_with_links(obj_deps)"},{"line_number":72,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_d1d2c633","line":69,"in_reply_to":"3f79a3b5_6115483d","updated":"2018-09-26 10:08:00.000000000","message":"Yes, agree with you. But there is no reserve function in cyborg for now as I know, so I put it here. It\u0027s a temporary solution. Please let me know if I miss something.","commit_id":"9f90465df75f99e41e17b668fb61d604cb851182"},{"author":{"_account_id":27458,"name":"Li Liu","email":"liliueecg@gmail.com","username":"liliu"},"change_message_id":"0e66a5d0f6c43e496f7d2ea5ab8ad70a64393b44","unresolved":false,"context_lines":[{"line_number":66,"context_line":"                obj_deps \u003d obj_deps + deps[:res_num]"},{"line_number":67,"context_line":"        for obj in obj_deps:"},{"line_number":68,"context_line":"            obj[\"instance_uuid\"] \u003d instance_uuid"},{"line_number":69,"context_line":"            obj[\"assignable\"] \u003d False"},{"line_number":70,"context_line":"            pecan.request.conductor_api.deployable_update(context, obj)"},{"line_number":71,"context_line":"        return DeployableCollection.convert_with_links(obj_deps)"},{"line_number":72,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_6d3a6bc4","line":69,"in_reply_to":"3f79a3b5_d1d2c633","updated":"2018-10-03 01:42:38.000000000","message":"If it is a temp solution, please mark a #TODO for future reference.","commit_id":"9f90465df75f99e41e17b668fb61d604cb851182"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"73e9eaa101db455291d64c8c27bb1a5eb1da7913","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        for obj in obj_deps:"},{"line_number":68,"context_line":"            obj[\"instance_uuid\"] \u003d instance_uuid"},{"line_number":69,"context_line":"            obj[\"assignable\"] \u003d False"},{"line_number":70,"context_line":"            pecan.request.conductor_api.deployable_update(context, obj)"},{"line_number":71,"context_line":"        return DeployableCollection.convert_with_links(obj_deps)"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    @policy.authorize_wsgi(\"cyborg:actions:deployable\", \"deallocation\", False)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_012494b1","line":70,"updated":"2018-09-24 22:45:22.000000000","message":"it would be better to do a bulk update of the conductor ranther then invoking multipe RPCs.","commit_id":"9f90465df75f99e41e17b668fb61d604cb851182"},{"author":{"_account_id":25738,"name":"Xinran WANG","email":"xin-ran.wang@intel.com","username":"Xinran"},"change_message_id":"45726cf4274101032c594ebeea6922021e7be58c","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        for obj in obj_deps:"},{"line_number":68,"context_line":"            obj[\"instance_uuid\"] \u003d instance_uuid"},{"line_number":69,"context_line":"            obj[\"assignable\"] \u003d False"},{"line_number":70,"context_line":"            pecan.request.conductor_api.deployable_update(context, obj)"},{"line_number":71,"context_line":"        return DeployableCollection.convert_with_links(obj_deps)"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    @policy.authorize_wsgi(\"cyborg:actions:deployable\", \"deallocation\", False)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_f1eca2f5","line":70,"in_reply_to":"3f79a3b5_012494b1","updated":"2018-09-26 10:08:00.000000000","message":"yes, will fix it soon, thanks.","commit_id":"9f90465df75f99e41e17b668fb61d604cb851182"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"73e9eaa101db455291d64c8c27bb1a5eb1da7913","unresolved":false,"context_lines":[{"line_number":71,"context_line":"        return DeployableCollection.convert_with_links(obj_deps)"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    @policy.authorize_wsgi(\"cyborg:actions:deployable\", \"deallocation\", False)"},{"line_number":74,"context_line":"    @lockutils.synchronized(\u0027deallocation\u0027, \u0027cyborg-\u0027)"},{"line_number":75,"context_line":"    @expose.expose(None, types.uuid, types.jsontype)"},{"line_number":76,"context_line":"    def deallocation(self, instance_uuid\u003dNone, deployable_uuids\u003dNone):"},{"line_number":77,"context_line":"        context \u003d pecan.request.context"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_21299088","line":74,"updated":"2018-09-24 22:45:22.000000000","message":"same as above i dont think this lock is a good idea form a perfromace standpoint as it would effectvly mean we could only dealocate devices serialy across the cloud","commit_id":"9f90465df75f99e41e17b668fb61d604cb851182"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"73e9eaa101db455291d64c8c27bb1a5eb1da7913","unresolved":false,"context_lines":[{"line_number":73,"context_line":"    @policy.authorize_wsgi(\"cyborg:actions:deployable\", \"deallocation\", False)"},{"line_number":74,"context_line":"    @lockutils.synchronized(\u0027deallocation\u0027, \u0027cyborg-\u0027)"},{"line_number":75,"context_line":"    @expose.expose(None, types.uuid, types.jsontype)"},{"line_number":76,"context_line":"    def deallocation(self, instance_uuid\u003dNone, deployable_uuids\u003dNone):"},{"line_number":77,"context_line":"        context \u003d pecan.request.context"},{"line_number":78,"context_line":"        obj_deps \u003d []"},{"line_number":79,"context_line":"        if not instance_uuid and not deployable_uuids:"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_c11d3c63","line":76,"updated":"2018-09-24 22:45:22.000000000","message":"as with the allocate funciton this feels like you are leaking db logic into \nthe api layer. this really suould be delegating all the db logic to a function in a serperate module. this fuction is effectivly performaning 2 sql commands\nfirst it retrives all deployable by instance uuid and then serialy updates the consuming insatnace uuid to null and sets the the deployable as available.\n\nthis could be implemented as a single sql command.\n\nupdate deployable set instance_uuid\u003dNone, assignable\u003d\"True\" where instance_uuid\u003d\"\u003cuuid\u003e\";\n\ni would suggest factoring this out into a stored procedure like objects.Deployable.get_by_filter\n\ne.g. objects.Deployable.release_by_uuid(instance_uuid)\n\nthen rely on the condoctor healing by rereading the database later\nor sending a notification for it to refresh its selfe passing the instance uuid","commit_id":"9f90465df75f99e41e17b668fb61d604cb851182"},{"author":{"_account_id":25738,"name":"Xinran WANG","email":"xin-ran.wang@intel.com","username":"Xinran"},"change_message_id":"45726cf4274101032c594ebeea6922021e7be58c","unresolved":false,"context_lines":[{"line_number":73,"context_line":"    @policy.authorize_wsgi(\"cyborg:actions:deployable\", \"deallocation\", False)"},{"line_number":74,"context_line":"    @lockutils.synchronized(\u0027deallocation\u0027, \u0027cyborg-\u0027)"},{"line_number":75,"context_line":"    @expose.expose(None, types.uuid, types.jsontype)"},{"line_number":76,"context_line":"    def deallocation(self, instance_uuid\u003dNone, deployable_uuids\u003dNone):"},{"line_number":77,"context_line":"        context \u003d pecan.request.context"},{"line_number":78,"context_line":"        obj_deps \u003d []"},{"line_number":79,"context_line":"        if not instance_uuid and not deployable_uuids:"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_5136b651","line":76,"in_reply_to":"3f79a3b5_c11d3c63","updated":"2018-09-26 10:08:00.000000000","message":"agree, will move these logic to object layer, thanks for you suggestion.","commit_id":"9f90465df75f99e41e17b668fb61d604cb851182"}],"cyborg/api/controllers/v1/deployable/model.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"73e9eaa101db455291d64c8c27bb1a5eb1da7913","unresolved":false,"context_lines":[{"line_number":13,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":14,"context_line":"#    under the License."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"import json"},{"line_number":17,"context_line":"import pecan"},{"line_number":18,"context_line":"import wsme"},{"line_number":19,"context_line":"from wsme import types as wtypes"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_e109582b","line":16,"updated":"2018-09-24 22:45:22.000000000","message":"you might want to use oslo serialisations json utils here instaed\nof the json lib directly.","commit_id":"9f90465df75f99e41e17b668fb61d604cb851182"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"73e9eaa101db455291d64c8c27bb1a5eb1da7913","unresolved":false,"context_lines":[{"line_number":16,"context_line":"import json"},{"line_number":17,"context_line":"import pecan"},{"line_number":18,"context_line":"import wsme"},{"line_number":19,"context_line":"from wsme import types as wtypes"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"from cyborg.api.controllers import base"},{"line_number":22,"context_line":"from cyborg.api.controllers import link"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_810e6431","line":19,"updated":"2018-09-24 22:45:22.000000000","message":"similarlay you proably should prefer\nusing types form https://github.com/openstack/oslo.versionedobjects/blob/master/oslo_versionedobjects/fields.py\nover a thridparty typeing lib since\nthe ovo lib is already a depency of cyborg.","commit_id":"9f90465df75f99e41e17b668fb61d604cb851182"}],"cyborg/common/policy.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"73e9eaa101db455291d64c8c27bb1a5eb1da7913","unresolved":false,"context_lines":[{"line_number":135,"context_line":""},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"def list_policies():"},{"line_number":138,"context_line":"    return default_policies \\"},{"line_number":139,"context_line":"        + accelerator_policies \\"},{"line_number":140,"context_line":"        + deployable_policies \\"},{"line_number":141,"context_line":"        + fpga_policies \\"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_c1045c17","line":138,"updated":"2018-09-24 22:45:22.000000000","message":"it would ba signifcantly more memory efficent to use itertool.chain to return\nan iterable over all the static list then generate a new list by contatonating all\nthe sub list every time list_policies is called.","commit_id":"9f90465df75f99e41e17b668fb61d604cb851182"},{"author":{"_account_id":25738,"name":"Xinran WANG","email":"xin-ran.wang@intel.com","username":"Xinran"},"change_message_id":"45726cf4274101032c594ebeea6922021e7be58c","unresolved":false,"context_lines":[{"line_number":135,"context_line":""},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"def list_policies():"},{"line_number":138,"context_line":"    return default_policies \\"},{"line_number":139,"context_line":"        + accelerator_policies \\"},{"line_number":140,"context_line":"        + deployable_policies \\"},{"line_number":141,"context_line":"        + fpga_policies \\"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_514d56c3","line":138,"in_reply_to":"3f79a3b5_c1045c17","updated":"2018-09-26 10:08:00.000000000","message":"maybe we should submit another patch about this?","commit_id":"9f90465df75f99e41e17b668fb61d604cb851182"}]}