| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-cyborg - 2020-04-16 | |||
| 13:48:51 | openstackgerrit | YumengBao proposed openstack/cyborg master: Fix bandit error: [B108:hardcoded_tmp_directory] https://review.opendev.org/720143 | |
| 13:48:51 | openstackgerrit | YumengBao proposed openstack/cyborg master: Fix bandit error: [B104:hardcoded_bind_all_interfaces] https://review.opendev.org/720149 | |
| 13:48:52 | openstackgerrit | YumengBao proposed openstack/cyborg master: Fix bandit error: Ascend driver:[B602:subprocess_popen_with_shell_equals_true] https://review.opendev.org/720456 | |
| 13:48:52 | openstackgerrit | YumengBao proposed openstack/cyborg master: Fix bandit error: SPDK driver:[B602:subprocess_popen_with_shell_equals_true] https://review.opendev.org/720475 | |
| 13:48:53 | openstackgerrit | YumengBao proposed openstack/cyborg master: Change bandit job from non-voting to voting https://review.opendev.org/720479 | |
| #openstack-cyborg - 2020-04-17 | |||
| 00:10:44 | openstackgerrit | Brin Zhang proposed openstack/cyborg master: revert device and deployable when resource provider create fail https://review.opendev.org/718584 | |
| 03:14:28 | openstackgerrit | YumengBao proposed openstack/cyborg master: Fix bandit error: SPDK driver:[B602:subprocess_popen_with_shell_equals_true] https://review.opendev.org/720475 | |
| 07:19:58 | openstackgerrit | YumengBao proposed openstack/cyborg master: Fix bandit error: [B108:hardcoded_tmp_directory] https://review.opendev.org/720143 | |
| 18:38:54 | openstackgerrit | Andreas Jaeger proposed openstack/python-cyborgclient master: Update docs building https://review.opendev.org/720804 | |
| 18:39:25 | openstackgerrit | Andreas Jaeger proposed openstack/python-cyborgclient master: Update docs building https://review.opendev.org/720804 | |
| #openstack-cyborg - 2020-04-20 | |||
| 06:01:41 | openstackgerrit | Brin Zhang proposed openstack/cyborg master: hacking: force explicit import of python's mock https://review.opendev.org/716920 | |
| #openstack-cyborg - 2020-04-21 | |||
| 11:51:19 | openstackgerrit | YumengBao proposed openstack/cyborg master: Fix bandit error: [B108:hardcoded_tmp_directory] https://review.opendev.org/720143 | |
| 11:56:25 | openstackgerrit | YumengBao proposed openstack/cyborg master: Change bandit job from non-voting to voting https://review.opendev.org/720479 | |
| 12:04:33 | openstackgerrit | YumengBao proposed openstack/cyborg master: Change bandit job from non-voting to voting https://review.opendev.org/720479 | |
| #openstack-cyborg - 2020-04-22 | |||
| 10:26:03 | openstackgerrit | Brin Zhang proposed openstack/cyborg master: revert device and deployable when resource provider create fail https://review.opendev.org/718584 | |
| 11:43:00 | openstackgerrit | Merged openstack/cyborg master: revert device and deployable when resource provider create fail https://review.opendev.org/718584 | |
| #openstack-cyborg - 2020-04-23 | |||
| 03:01:44 | Sundar | Hi all | |
| 03:01:58 | Sundar | #startmeeting openstack-cyborg | |
| 03:01:59 | openstack | Meeting started Thu Apr 23 03:01:58 2020 UTC and is due to finish in 60 minutes. The chair is Sundar. Information about MeetBot at http://wiki.debian.org/MeetBot. | |
| 03:02:00 | openstack | Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. | |
| 03:02:02 | openstack | The meeting name has been set to 'openstack_cyborg' | |
| 03:02:09 | s_shogo | Hi all | |
| 03:02:13 | songwenping_ | Hi all | |
| 03:02:28 | xinranwang | Hi all | |
| 03:03:21 | Yumeng | #info Yumeng | |
| 03:03:24 | brinzhang | hi all | |
| 03:03:28 | Yumeng | hi all | |
| 03:03:40 | Sundar | Good, we have a quorum. | |
| 03:04:04 | Sundar | Does anybody have a specific thing to discuss? | |
| 03:04:09 | Sundar | *any | |
| 03:04:10 | chenke | hi all | |
| 03:04:13 | chenke | #info chenke | |
| 03:04:23 | xinranwang | #info xinranwang | |
| 03:04:40 | s_shogo | #info s_shogo | |
| 03:05:08 | brinzhang | #info brinzhang | |
| 03:06:04 | Yumeng | hi all, pls help to review and merge https://review.opendev.org/#/q/topic:fix-bandit-check-failures+(status:open+OR+status:merged) | |
| 03:06:07 | brinzhang | I have some summary work in my company, so there few work take here in this meeting, I am sorry | |
| 03:06:38 | brinzhang | Yumeng: I reviewed the first patch, I hope you can add the test case to cover your changes | |
| 03:06:43 | Sundar | Looking at https://review.opendev.org/#/q/status:open+project:openstack/cyborg+branch:master | |
| 03:06:48 | Yumeng | brinzhang: I just replied your comments: https://review.opendev.org/#/c/720143/4/cyborg/agent/manager.py | |
| 03:06:55 | Sundar | Agree with Yumeng -- bandit fixes are important | |
| 03:07:22 | Sundar | I'll review them after this meeting | |
| 03:07:30 | brinzhang | Yumeng: I mean, you should add test case to test https://review.opendev.org/#/c/720143/4/cyborg/agent/manager.py@52 | |
| 03:07:31 | Yumeng | Sundar: Thanks! | |
| 03:07:37 | brinzhang | the fpga_program_v2() | |
| 03:08:34 | brinzhang | We can’t give up adding new use cases because they do n’t affect existing use cases, can we? | |
| 03:08:46 | Sundar | Also, I'd say the functional tests patch https://review.opendev.org/#/c/702863/ is important | |
| 03:09:20 | Yumeng | brinzhang: I know. fpga_program_v2() here just made a temp file and downloaded the image, then removed the file. it's more like a file operation. If we add unit test for fpga_program_v2(), they are all by mock. | |
| 03:09:49 | brinzhang | Sundar, there is an issue, that cannot work successful in my local, and I havenot found the reason now | |
| 03:09:54 | Yumeng | brinzhang: that's why at the beginnig , we didn't add unit test for this func | |
| 03:10:13 | brinzhang | if anyone good at functional test, please give some help ^, thanks | |
| 03:10:25 | Sundar | brinzhang: Are you saying the functional tests do not run in your local env, or they run but do not pass? | |
| 03:10:54 | brinzhang | Sundar, that why I am confusing, so I give -1 in this patch | |
| 03:11:34 | brinzhang | you can run tox -e functional in you local env, and test. The test cannot cover the functional dir too. | |
| 03:11:36 | xinranwang | I looked into brinzhang's functional test issue, It seems that all funtional tests code are not loaded successfully. | |
| 03:11:49 | brinzhang | xinranwang: right | |
| 03:12:53 | brinzhang | Yumeng: I understand, the UT mainly keep the function work fine, mock data is enough, I think | |
| 03:12:57 | shaohe_feng | the problem is: str + UUID ? | |
| 03:14:23 | brinzhang | shaohe_feng: yes, I reviewed your comments in PS3, and left some comments in latest patch | |
| 03:14:55 | shaohe_feng | Yumeng have check the type of uuid? seems chenke has checked it, already str format. | |
| 03:15:16 | chenke | I had alerady check it. str format. | |
| 03:15:31 | chenke | But functional test may be some error about this type. | |
| 03:16:12 | shaohe_feng | strangely. | |
| 03:16:38 | chenke | From the logical point of view of the code, when calling the method, uuid is read from the device profile, which is itself str, and we do not need to do conversion. | |
| 03:16:39 | Yumeng | shaohe_feng: yes, uuid. shogo has helped check this patch in his real FPGA env. | |
| 03:17:19 | shaohe_feng | maybe not the source code error | |
| 03:17:37 | shaohe_feng | something wrong it the functional test code. | |
| 03:17:38 | brinzhang | chenke: we also should keep the UT can works fine | |
| 03:17:40 | chenke | @brin . Did you encounter the problem when running the fun test? | |
| 03:17:54 | shaohe_feng | s/something wrong it the functional test code. | |
| 03:17:58 | Yumeng | if not uuid, it should have raised an error | |
| 03:18:03 | chenke | I guess. | |
| 03:18:29 | brinzhang | chenke: I paste my test step, you can review again | |
| 03:19:06 | chenke | ok. | |
| 03:19:31 | brinzhang | Yumeng: so, there will be negative and active scenario need to be test, if the bitstream_uuid is str, or not str | |
| 03:19:32 | Sundar | Got disconnected when I logged into my VPN | |
| 03:19:33 | s_shogo | I also check the test test of brin. | |
| 03:19:49 | s_shogo | s/test test/test step | |
| 03:22:15 | Yumeng | brinzhang: I think the mian difference is this step of your test step: bitstream_uuid=uuid.uuid1(). this step manually set the bitstream_uuid type to uuid object | |
| 03:22:34 | Sundar | Yumeng: In https://review.opendev.org/720475, do we want to run bash with some unknown variable as argument? That can be insecure too. | |
| 03:22:57 | brinzhang | if there is a UT, that we can see clear their different | |
| 03:25:34 | chenke | actually. UT for this method, only can verify the logic of method. The UT will not ensure the type of uuid. We can not expect UT and do everything for us. It can help our code more strongly. | |
| 03:26:22 | chenke | s/and/can/ | |
| 03:27:20 | Sundar | To check if a value is a UUID, we can do: try: | |
| 03:27:43 | Sundar | There must be some oslo check for it too. Checking now ... | |
| 03:27:56 | Yumeng | Sundar: emmm.. seems in spdk start_server() function, we cannot avoid the server_name (unkown variable) as argument , but without shell=True is much better than before | |
| 03:28:12 | brinzhang | Sundar: what do your consider? I am not very familiar with bandit | |
| 03:28:46 | Sundar | For UUID: oslo_utils.uuidutils.is_uuid_like(val) | |
| 03:28:57 | chenke | As we often hear, practice is the only criterion for testing truth. I think your test is very good. If necessary, I think we can add a uuid type check. If it doesn’t pass, throw an exception. Do you think this is a good idea? | |
| 03:29:16 | Yumeng | Sundar: I will talk to Li Liu, and see if he has better suggestion. I will sync you later. | |
| 03:29:34 | brinzhang | Sundar: yes, this just only check the parameter is a uuid, not determine it's type | |
| 03:29:45 | shaohe_feng | can we double check to make make the uuid is srting | |
| 03:29:48 | shaohe_feng | string | |
| 03:29:50 | Sundar | self.assertTrue( oslo_utils.uuidutils.is_uuid_like(val) ) | |
| 03:29:55 | brinzhang | so add UT to cover this change, do you agree? | |
| 03:30:16 | songwenping_ | Just looking at this patch. Brin's error occurred in _os.path.join(dir, pre + name + suf). Should we check the name type? | |
| 03:30:18 | shaohe_feng | in | |
| 03:31:20 | shaohe_feng | format again str(uuid) | |
| 03:31:23 | chenke | agree and a type check about uuid. and add a test to cover this check situation. | |
| 03:32:28 | shaohe_feng | IMHO, a good function should can handle both UUID or str type both | |