| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-cyborg - 2020-03-03 | |||
| 01:50:30 | chenke | Hi | |
| 01:50:42 | chenke | https://review.opendev.org/#/c/673735/40/nova/tests/unit/compute/test_compute_mgr.py | |
| 01:51:03 | chenke | Do your means Dan's advice will report error? | |
| 01:51:08 | Sundar | Yes | |
| 01:51:28 | chenke | You can write like this:https://github.com/openstack/cyborg/blob/acbc64f3bedc4f4946c5d12ea4fd7dc3d941667a/cyborg/tests/unit/db/test_db_device_profile.py#L85-L87 | |
| 01:51:50 | Sundar | That's what I did | |
| 01:52:19 | Sundar | It fails saying it throws AcceleratorOp exception. | |
| 01:53:11 | Sundar | What is wrong with the current test? I am not trying to check that the code raises an exception. But the test case mocking will force an exception. So, we shouldn't need to assert for that. | |
| 01:55:32 | chenke | Emmm. Before that, I thought you were unfamiliar with the use of assertRaises, but it wasn't actually the problem. | |
| 01:56:16 | chenke | It seems I still need to read the code carefully. | |
| 01:58:27 | Sundar | ok, np, thanks | |
| 02:02:32 | chenke | About line 6289,6290 with self.compute._build_resources(*args): | |
| 02:02:47 | chenke | I think you should delete this. | |
| 02:06:43 | chenke | just remove the line raise exception.NovaException. | |
| 02:07:20 | openstackgerrit | Merged openstack/cyborg master: Remove useless interfaces in cond https://review.opendev.org/710553 | |
| 02:22:22 | Sundar | chenke: self.compute._build_resources is a context manager.When an exception is thrown inside the with statement, that causes a specific code path inside _build_resources_, which is where AcceleratorOp excepotion gets raised. | |
| 02:23:25 | chenke | I know. I means write like this: self.compute._build_resources(*args) | |
| 02:25:05 | chenke | I think then this method will raise AcceleratorOp excepotion. And in line 6298 we can asertRaises(AcceleratorOp excepotion) | |
| 02:32:18 | Sundar | I tried it. It fails with: AssertionError: Expected 'delete_arqs_if_needed' to be called once. Called 0 times. | |
| 02:32:45 | Sundar | IOW, we need to raise an exception inside the with to force the delete_arqs_if_needed to be called. | |
| 02:40:19 | chenke | If we comment lin3 6301, The ut will pass, right? | |
| 02:56:16 | chenke | Dan's advice I think is to let you modify Line 6290 to use another exception instead of novaException which will confuse others. | |
| 03:47:03 | Sundar | I fixed it in a different way: http://paste.openstack.org/show/790224/ | |
| 04:24:55 | chenke | Ok | |
| 04:28:00 | chenke | https://review.opendev.org/#/c/673735/40..41/nova/tests/unit/compute/test_compute_mgr.py@6293 | |
| 04:28:42 | chenke | https://review.opendev.org/#/c/673735/40..41/nova/tests/unit/compute/test_compute_mgr.py@6293 | |
| 04:29:01 | Sundar | Hi chenke | |
| 04:29:08 | chenke | I think this can write like this: | |
| 04:30:06 | chenke | remove try...except, just self.assertRaise(exception.BuildAbortException, self.compute._build_resources....) | |
| 04:30:24 | Sundar | Hmm, let me try that | |
| 04:30:33 | chenke | ok | |
| 04:34:16 | Sundar | Fails with: testtools.matchers._impl.MismatchError: <bound method ComputeManager._build_resources of <nova.compute.manager.ComputeManager object at 0x7f3f25852a90>> returned <contextlib._GeneratorContextManager object at 0x7f3f2521fb00> | |
| 04:34:31 | Sundar | This is expected because _build_resources is a context manager | |
| 04:34:40 | Sundar | I think Dan is wrong with that advice | |
| 04:37:15 | Sundar | Here's my diff; http://paste.openstack.org/show/790226/ | |
| 04:37:20 | Sundar | You can also try it | |
| 04:37:30 | Sundar | chenke: ^ | |
| 06:29:18 | chenke | The paste diff result failed? right? | |
| 06:35:43 | Sundar | Yes, chenke | |
| 06:35:54 | chenke | Ok. | |
| 06:36:18 | chenke | That's strange. | |
| 06:37:51 | chenke | It's means self.compute._build_resources will not raise an exception? | |
| 06:57:29 | chenke | Sundar are you here? | |
| 06:57:31 | openstackgerrit | chenker proposed openstack/cyborg master: Remove useless interfaces in agent https://review.opendev.org/710911 | |
| 07:00:55 | openstackgerrit | chenker proposed openstack/cyborg master: Remove useless interfaces in agent https://review.opendev.org/710911 | |
| 07:07:17 | Sundar | Yes, I am here, chenke | |
| 07:07:27 | chenke | Cool | |
| 07:07:37 | chenke | It's means self.compute._build_resources will not raise an exception? | |
| 07:07:38 | Sundar | Just saw your message above | |
| 07:07:52 | chenke | NP | |
| 07:08:15 | Sundar | Yes, for a context manager decorated function, we need to invoke the exception inside the with body, to trigger the eception path inside the function | |
| 07:08:54 | chenke | Oh. Cool. | |
| 07:09:23 | chenke | The latest patch seems UT passed. | |
| 07:09:38 | chenke | I will give +1 after dan's review. | |
| 07:12:56 | openstackgerrit | chenker proposed openstack/cyborg master: Remove useless interfaces in agent https://review.opendev.org/710911 | |
| 07:14:48 | openstackgerrit | Merged openstack/cyborg master: move setting to devstack/settings https://review.opendev.org/708770 | |
| 07:18:00 | openstackgerrit | chenker proposed openstack/cyborg master: Remove useless interfaces in agent https://review.opendev.org/710911 | |
| 07:26:57 | Sundar | Sure. Thanks, chenke. | |
| 07:27:17 | chenke | :) | |
| 07:34:15 | Sundar | Going to sleep now. Talk to you later. Have a good day. Bye! | |
| 17:10:30 | openstackgerrit | Dan Smith proposed openstack/cyborg master: WIP add cyborg multinode job https://review.opendev.org/709641 | |
| #openstack-cyborg - 2020-03-04 | |||
| 13:18:09 | openstackgerrit | chenker proposed openstack/cyborg master: Optimize deployble object module https://review.opendev.org/711222 | |
| 13:27:19 | openstackgerrit | chenker proposed openstack/cyborg master: Fix minor error in extarq job https://review.opendev.org/711225 | |
| 13:39:25 | openstackgerrit | chenker proposed openstack/cyborg master: (WIP) Remove V1 API and related codes https://review.opendev.org/711232 | |
| 13:45:07 | openstackgerrit | chenker proposed openstack/cyborg master: (WIP) Remove V1 API and related codes https://review.opendev.org/711232 | |
| 14:14:33 | openstackgerrit | chenker proposed openstack/cyborg master: Fix minor error in extarq job https://review.opendev.org/711225 | |
| #openstack-cyborg - 2020-03-05 | |||
| 01:44:11 | openstackgerrit | chenker proposed openstack/cyborg master: Remove V1 API and related codes https://review.opendev.org/711232 | |
| 02:52:49 | brinzhang | Sundar: ask a question https://review.opendev.org/#/c/711225/2/cyborg/objects/extarq/ext_arq_job.py@39 | |
| 02:53:12 | Sundar | brinzhang: Ok, looking at it | |
| 02:53:43 | brinzhang | Sundar: "if getattr(self.bind, "is_job", False) and need_job is not False:" if need_job=False, is it what would you want? | |
| 02:53:50 | Sundar | What is the question? | |
| 02:54:24 | brinzhang | Sundar: check_extra_job(context, deployable) this just only return True or False? or there is other volue? | |
| 02:55:04 | brinzhang | Dou you accept current changes? | |
| 02:55:15 | brinzhang | s/Dou/Do | |
| 02:56:22 | brinzhang | "need_job is not False" has the different value when need_job init None or False | |
| 02:58:32 | Sundar | IMHO, need_job should be boolean (True/False). The check should be: "if need_job: ..." | |
| 02:59:07 | Sundar | Having a possible value of None just complicates other code that has to check for None. | |
| 02:59:39 | brinzhang | agree | |
| 03:00:00 | Sundar | So, I agree with this change. | |
| 03:00:19 | brinzhang | so "if getattr(self.bind, "is_job", False) and need_job is not False:" should change to "if getattr(self.bind, "is_job", False) and need_job:" | |
| 03:01:18 | Sundar | Yes, agree | |
| 03:01:31 | chenke | need_job or need_job is not False both ok. | |
| 03:01:57 | brinzhang | And I think we need add some UT for this, now the change dont run some UT | |
| 03:01:58 | Sundar | chenke: Sure. But shorter is better. | |
| 03:02:18 | Sundar | Looks like we have quite a few people. So, let's get started. | |
| 03:02:25 | chenke | Ok. | |
| 03:02:31 | Sundar | #startmeeting openstack-cyborg | |
| 03:02:32 | openstack | Meeting started Thu Mar 5 03:02:31 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:33 | openstack | Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. | |
| 03:02:35 | openstack | The meeting name has been set to 'openstack_cyborg' | |
| 03:02:47 | Sundar | #topic Agenda | |
| 03:02:55 | chenke | #info chenke | |
| 03:03:04 | Sundar | #info Sundar | |
| 03:03:14 | Li_Liu | #info Li_Liu | |
| 03:03:29 | brinzhang | #info brinzhang | |
| 03:03:36 | Yumeng | #info Yumeng | |
| 03:04:10 | xinranwang | #info xinranwang | |
| 03:04:26 | Sundar | What do people have to discuss today? | |
| 03:05:18 | brinzhang | About remove v1 API | |
| 03:05:19 | brinzhang | https://review.opendev.org/#/c/711232/ | |