| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-cyborg - 2019-08-22 | |||
| 03:05:09 | Sundar | #topic Python 3 | |
| 03:05:54 | Sundar | There has been some feedback from TC to fix Python 3 tests and make them voting jobs | |
| 03:06:10 | Sundar | We should minimally try to get the Python 3 stuff fixe | |
| 03:06:12 | Sundar | *fixed | |
| 03:06:37 | Sundar | s_shogo: Thanks for your patches. Is it possible to prioritize it? We can even test with the patches | |
| 03:07:08 | xinranwang | #info xinranwang | |
| 03:07:52 | s_shogo | I'm waiting for merge of P1-P9 patches, after that , I'll delete WIP in py3 patch. | |
| 03:08:47 | Sundar | We can test and update the Py3 patch with the P5-P9 patches. I agree we need to speed up the merging of those patches. But we don't need to wait for that to update Py3 fixes, right? | |
| 03:09:23 | s_shogo | ok, I'll delete the WIP and request review today. | |
| 03:09:48 | Coco_gao_ | Thanks | |
| 03:10:12 | s_shogo | I intend to commit the patch with "non-voting" ,are there any problems? | |
| 03:10:56 | Sundar | We all voted in a past IRC meeting that it should be non-voting for now. So I agree. | |
| 03:11:08 | s_shogo | Ok. | |
| 03:11:09 | Sundar | Do you need help with testing with the patches? You could do: 'git review -d 670470' and then 'git review -x 667524' | |
| 03:11:26 | Sundar | That will pull the P5-P9 patches and your patch in one branch | |
| 03:12:02 | s_shogo | Thanks, I'll do that. | |
| 03:12:13 | Sundar | Great. Thanks a lot, s_shogo :) | |
| 03:12:19 | Sundar | #topic Merging patches | |
| 03:12:48 | Sundar | As s_shogo pointed out, a lot of things are blocked on merging the P5-P9 patches. Can you all please prioritize that? | |
| 03:13:05 | Coco_gao_ | Agree | |
| 03:13:20 | wangzhh | Sure. | |
| 03:14:09 | Coco_gao_ | About P5 patch, a lot of patch is denpending on it. | |
| 03:14:23 | Sundar | Great. Xinran's patches on Nova notification and Placement are important too. Once they also merge, we can enable tempest CI, and that is what we need to kick off Nova integ. | |
| 03:14:58 | Yumeng | hi Sundar: I noticed you gave a -2 on P5: Basic changes for API layer. https://review.opendev.org/#/c/670466/ are there any problems? | |
| 03:15:00 | Coco_gao_ | yumeng also need to modify the database | |
| 03:15:00 | Sundar | Coco_gao: Agreed. I have tested the P5-P9 with fake and FPGA drivers. | |
| 03:15:44 | xinranwang | Sundar: do you want to discuss vendor_id, vendor_name stuff, which we discussed yesterday? | |
| 03:15:59 | Sundar | Yumeng: It is a procedural -2 to prevent the patch series from merging piece by piece. It will be cleaner if we all review all the patches as a whole and then the whole thing merges around the same time. | |
| 03:16:22 | Sundar | Same reasoning as with the -2 for https://review.opendev.org/631242 in Nova | |
| 03:17:05 | xinranwang | Please review placement report patch, it has less dependency on P5-P6 patch. :) | |
| 03:17:11 | Yumeng | Sundar: got it . thanks | |
| 03:17:14 | Coco_gao_ | So can we make the priority list about the patches? | |
| 03:18:15 | Sundar | Coco_gao: I would suggest: P5-P9, Nova notification, Placement report. What do you all think? | |
| 03:18:32 | Sundar | Not saying other patches are less important :) | |
| 03:18:51 | Sundar | Just tryin to unblock as much as we can and get the ball back to Nova developers' court. | |
| 03:19:17 | Coco_gao_ | I think some of the patches had dependency issues so I am asking about the priority. | |
| 03:19:22 | xinranwang | I think we can review placement from now, cause it has less dependency on your patches. | |
| 03:19:47 | xinranwang | notification after p5-p9 | |
| 03:20:05 | Coco_gao_ | Both dependency and our Train release goal are matter. | |
| 03:20:12 | Sundar | xinranwang: Not disagreeing, but Placement needs to be tested with P5-P9 patches, right? | |
| 03:20:24 | xinranwang | No | |
| 03:20:46 | Coco_gao_ | P5-P9, Nova notification, Placement report. What about python3? | |
| 03:20:49 | xinranwang | we can run placement patch without nova-cyborg integration | |
| 03:21:29 | Sundar | Ok, we could do partial testing with CYborg driver -> agent -> conductor (db) -> Placement. Without using Nova. Is that what you are saying? | |
| 03:21:44 | xinranwang | Yes, exactl. | |
| 03:21:51 | xinranwang | *exactly | |
| 03:21:58 | Coco_gao_ | OK, got that. | |
| 03:22:00 | Sundar | Agreed. | |
| 03:22:35 | xinranwang | Also, placement provides cmd line like openstack resource provider list, we can check the result directly by using this. | |
| 03:23:11 | Sundar | Ok, then order with dependencies is: [ P5-P9, Placement report], then nova notification. Sounds good? | |
| 03:23:31 | Coco_gao_ | Great. | |
| 03:23:34 | chenke | yes. | |
| 03:24:16 | xinranwang | yes, great | |
| 03:24:17 | Coco_gao_ | I see there are several patches which had conflict problems. | |
| 03:24:36 | Coco_gao_ | Will that be solved if we merge P5-P9 | |
| 03:25:17 | Sundar | Coco_gao_: In https://review.opendev.org/#/q/status:open+project:openstack/cyborg+branch:master, I see 4 merge conflicts and they are all old patches only for reference. Rest should be good. | |
| 03:25:18 | Coco_gao_ | Seems good to me because I know where to start my reviews. | |
| 03:26:13 | xinranwang | Sundar wangzhh: I noticed that GPU driver report vendor id and fill the vendor field with ID, is there any possibility that we fill vendor field by vendor name? | |
| 03:27:11 | Sundar | IMHO, we need the vendor name for the device somehwere, so that we can create the traits with the vendor name, as listed in https://review.opendev.org/gitweb?p=openstack/cyborg-specs.git;f=specs/train/approved/cyborg-nova-placement.rst;hb=refs/changes/45/603545/7#l179 | |
| 03:27:14 | Coco_gao_ | We can get the vendor name from vendor ID right? | |
| 03:27:44 | Coco_gao_ | Since the vendor ID is unique | |
| 03:28:01 | Sundar | Yes, but the driver report does not include the vendor name as a field today | |
| 03:28:14 | wangzhh | Of course, but I suggest record vendorId in db. And translate it to name if needed. | |
| 03:28:39 | Sundar | Perhaps it will be easiest to add vendor name as a separate field in the driver_objects.device, to minimize changes to GPU driver and other drivers | |
| 03:29:14 | Coco_gao_ | OK. | |
| 03:29:26 | Coco_gao_ | Agree that we should keep vendor ID | |
| 03:29:38 | xinranwang | each driver should know about the mapping relation between id and name, but cyborg-agent/conductor don't | |
| 03:30:11 | Sundar | The Cyborg driver for a vendor could even hardcode the vendor name, if it will handle only one vendor. | |
| 03:30:35 | xinranwang | So IMO, driver should report vendor name anyway. | |
| 03:30:54 | Sundar | xinranwang: Agreed. Proposal: Add vendor name to driver_objects.device . Any objections? | |
| 03:30:56 | wangzhh | Sundar, It is hardcoded in driver now. | |
| 03:31:08 | wangzhh | Agreed | |
| 03:31:26 | xinranwang | no objection. | |
| 03:31:45 | xinranwang | Btw, i find driver name in deployable tables, but no assignment. | |
| 03:32:04 | shaohe_feng | Yes， I translate the vendor ID to name for FPGA in my DEMO | |
| 03:32:05 | Coco_gao_ | Agree. The object.device will not add vendor name? Only driver_objects.device will? | |
| 03:32:54 | shaohe_feng | it can add a vendor name | |
| 03:33:24 | shaohe_feng | but maybe more driver infos, such as driver version. | |
| 03:33:24 | Sundar | Coco_gao_: Good point. It will be easier to add it to device object too, for use in ext-arq binding for creating traits | |
| 03:34:06 | Sundar | Do we need to add that to the db then? Oherwise, how how do we get it in the device object by querying? | |
| 03:34:07 | wangzhh | So just add name in object, not in db, right? | |
| 03:34:09 | Coco_gao_ | My question is do you think we should keep vendor name in object or database? | |
| 03:34:35 | shaohe_feng | for a long term. maybe we can translate the ID to name at present. | |
| 03:34:37 | shaohe_feng | DB | |
| 03:34:58 | Coco_gao_ | vendor ID is already in DB? | |
| 03:35:03 | xinranwang | Sundar: what is driver name you added in deployable table stand for? | |
| 03:35:22 | wangzhh | coco yep | |
| 03:35:27 | Sundar | If you don;t add the vendor name to db, the translation from id to name may have to be outside the driver too, which is not good | |
| 03:35:32 | xinranwang | Coco_gao_: yes, in device tables | |
| 03:36:09 | Yumeng | Coco_gao_:yes. already have vendor_id in db device table | |
| 03:36:42 | Coco_gao_ | About the ext-arq binding, do we need to qurey DB for vendor name? | |
| 03:36:44 | Sundar | xinranwang: It is the driver that is used for programming and device updates. At discovery time, we know which deployable has which driver. So, we can use that info to locate the driver needed for programming. | |
| 03:37:00 | shaohe_feng | https://github.com/shaohef/cyborg/commit/7e353bf09259a9b4cfba3763f3a99eb1adfaaa2b#diff-7ca60ed56d570ebe3490 | |
| 03:37:11 | xinranwang | Sundar: | |
| 03:37:20 | xinranwang | there is no assignment of this field | |
| 03:37:24 | Sundar | Coco_gao_: For ext_arq binding, we are queryig db for the deployable now. We can extend tthat to query the device and its vendor name too | |
| 03:37:38 | xinranwang | maybe we can extract vendor name from this field? | |
| 03:37:40 | shaohe_feng | here is what I do, just for a temporary solution | |
| 03:38:19 | shaohe_feng | driver_name = db_deployable.driver_name | |
| 03:39:44 | Sundar | xinranwang: The intention is that, when the agent discovers a device from a driver, it sends the driver name to the conductor and has it persisted. | |