Monday, 2016-01-18

openstackgerritQiming Teng proposed openstack/senlin: Fix receiver test case to make it stable  https://review.openstack.org/26878200:17
*** xuhaiwei has joined #senlin00:28
*** Yanyanhu has joined #senlin01:47
openstackgerritQiming Teng proposed openstack/senlin: Fix zone test case to make it stable  https://review.openstack.org/26878102:00
*** yuanying has quit IRC02:04
*** elynn has joined #senlin02:05
*** yuanying has joined #senlin02:05
openstackgerritQiming Teng proposed openstack/senlin: Fix receiver test case to make it stable  https://review.openstack.org/26878202:06
*** Qiming has joined #senlin02:06
*** elynn_ has joined #senlin02:12
*** elynn has quit IRC02:12
openstackgerritQiming Teng proposed openstack/senlin: Unit test for base profile  https://review.openstack.org/26875202:14
Qiminglixinhui, thanks for reviewing/approving https://review.openstack.org/26878102:27
openstackgerritMerged openstack/senlin: Unit test for base profile  https://review.openstack.org/26875202:27
Qiminghowever, that patch depends on https://review.openstack.org/26875202:27
Qimingand it is depended by https://review.openstack.org/26878202:28
lixinhuiQiming, I found it and just pass it02:28
Qimingthere is a critical error I want to discuss with you02:28
lixinhuiplease02:28
Qimingcheck this patch: https://review.openstack.org/#/c/268752/02:29
Qimingit took me a **long** time to find out the issue02:29
Qimingthe problem turns out to be in a test case here: https://review.openstack.org/#/c/268752/13/senlin/tests/unit/policies/test_vSphereDRSPolicy.py02:29
Qimingin 'test_attach_with_profile_info()' method02:30
Qimingthe first line is replacing the 'load' method of the Profile class, this is becoming permanent02:31
Qimingit is not the right way to mock a method call02:31
Qimingit is REPLACING the method call with a mock02:32
Qimingthe difference is a little bit subtle02:33
lixinhuiokay, we need to induce load testing02:35
lixinhuiinstead of direct asisignment of profile id02:35
Qimingwell, the lesson learnt is02:35
Qimingdon't change the behavior of a class method in that way02:36
Qimingit is okay to use mock.patch.object annotation02:36
Qimingit is also okay to do self.patchobject()02:36
Qimingbecause testtools will take care of the temporary behavior change02:36
Qimingthe line 'profile_base.Profile.load = mock.Mock()' is not a mock operation, it is permanently changing the class behavior02:37
lixinhuiyou mean, not only the return value is used, but some temporary change will cause the test failure02:37
lixinhuiokay02:38
QimingI mean don't do direct assignment on a method, do a mock patch02:38
Qimingit will forever change the behavior of the class02:38
lixinhuiI see02:39
Qimingso when other tests are executed after this one, they will get wrong behavior02:39
Qimingit is still fine to do something like this:02:40
Qimingobj = mock.Mock()02:40
Qimingobj.get_hypervisors = mock.Mock(return_value=something)02:40
Qimingbecause in the two lines above, you are changing the behavior of a mocked object02:41
lixinhuibut load is a behaviour02:43
Qimingyes02:43
Qimingit is a behavior you want to mock02:43
lixinhuiso once we made assgi ment, the class behaviour will change forever02:43
Qimingyes02:43
lixinhuiot it02:44
lixinhuigot it02:44
lixinhuiThanks, Qiming02:44
*** yuanying has quit IRC03:00
openstackgerritQiming Teng proposed openstack/senlin: Fix zone test case to make it stable  https://review.openstack.org/26878103:00
*** yuanying has joined #senlin03:01
Qiminggate is stalling due to dependencies03:01
Qimingelynn_, there?03:02
Qiminghelps needed on reviewing this: https://review.openstack.org/#/c/268563/03:06
Qimingit is needed by https://review.openstack.org/26878203:07
openstackgerritMerged openstack/senlin: Fix zone test case to make it stable  https://review.openstack.org/26878103:15
*** yuanying has quit IRC03:20
elynn_Hi Qiming03:20
Qiminghi, elynn_03:20
elynn_looking ~03:20
Qimingabout making 'senlin-api' and 'senlin-engine' a console script entry point03:20
Qiminghttps://review.openstack.org/#/c/267375/103:20
Qiminghttps://review.openstack.org/#/c/267376/103:21
elynn_yes03:21
Qimingcan you help check the reason behind the changes?03:21
QimingI know that some other projects are doing this03:21
Qimingbut we should know why this is recommended03:22
elynn_Maybe because pbr can handle it better? Let me check.03:22
Qiminggreat03:22
Qimingit looks like a tradeoff03:23
elynn_Why do you think so?03:23
Qimingkeeping service 'binaries' under 'bin' looks more intuitive03:23
Qiminghowever, when service is installed03:23
Qimingfor example, using devstack03:23
Qimingwe are getting some 'binaries' under /usr/bin03:24
Qimingthose new binaries are pointing to the scripts we have under senlin/bin03:24
Qimingif that is always the practice, then we can safely remove the senlin/bin directory03:25
Qimingand even the monkey patch thing in those scripts03:25
Qimingjust curious if this is the right direction to go03:25
elynn_I think it is, but I don't know why, let me google it :)03:27
Qimingwe are already hitting mitaka 2 milestone: http://docs.openstack.org/releases/schedules/mitaka.html03:27
Qimingif it is the RIGHT thing to do, maybe we should get it in NOW03:27
openstackgerritMerged openstack/senlin: Add unit tests for receiver in engine  https://review.openstack.org/26856303:28
elynn_Heat also add deprecation warning to binaries in bin/, so eventually bin will be removed.03:29
openstackgerritQiming Teng proposed openstack/senlin: Fix receiver test case to make it stable  https://review.openstack.org/26878203:29
Qimingokay?03:29
Qimingone thing I'd like to advocate across the team is that we don't do things simple because other projects are doing that03:30
Qimingwe do the right things03:30
Qimingso if we are making some changes, we'd better know the reason, :)03:30
openstackgerritQiming Teng proposed openstack/senlin: Fix receiver params string generation  https://review.openstack.org/26878203:32
elynn_yes, agree03:32
*** dixiaoli has joined #senlin03:34
openstackgerritLiuqing Jing proposed openstack/senlin: Ensure that jsonutils.%(fun)s must be used instead of json.%(fun)s  https://review.openstack.org/26264603:35
openstackgerritMerged openstack/senlin: Fix receiver params string generation  https://review.openstack.org/26878203:41
openstackgerritQiming Teng proposed openstack/senlin: Update text strings  https://review.openstack.org/26885303:42
openstackgerritLiuqing Jing proposed openstack/senlin: Ensure that jsonutils.%(fun)s must be used instead of json.%(fun)s  https://review.openstack.org/26264603:49
elynn_Hi Qiming, I don't find much evidence that we must move bin/ to console_scripts, but moving to console_scripts can take the advantage of setuptools and help us to avoid much errors according to https://review.openstack.org/#/c/190304/ .03:52
*** Liuqing has joined #senlin03:54
elynn_And we can also define wsgi_scripts in [entry_points] later if we want.04:02
*** yuanying has joined #senlin04:06
elynn_Some more info can be found here. http://docs.openstack.org/developer/pbr/#entry-points . Using pbr to handle console_scripts is more easy.04:06
*** elynn_ has quit IRC04:20
LiuqingHello, when i run `senlin profile-list`, the client raise error `AttributeError: 'Proxy' object has no attribute 'profiles'`04:41
LiuqingLOG is here : AttributeError: 'Proxy' object has no attribute 'profiles'04:41
Liuqingwhen i run another cmd `senlin cluster-list`, it raise `senlinclient.common.exc.HTTPInternalServerError: ERROR(500): 'Session' object has no attribute 'get_token'`04:42
QimingLiuqing, your openstack version is too old04:43
Qimingplease use master04:43
Liuqingyou mean openstack sdk ?04:44
Qimingyes04:44
Liuqingok04:44
Qiminggit clone it04:44
Qimingthen do a git install -e .04:44
Qimingsorry, 'sudo pip install -e .'04:45
*** elynn_ has joined #senlin04:45
LiuqingOK, thanks04:45
openstackgerritQiming Teng proposed openstack/senlin: Revise version json format  https://review.openstack.org/26886204:54
Qimingelynn_, checked the links05:00
Qiminglet's go deprecate the bin dir05:01
Qimingand we can remove the monkey patching logic lines then05:01
elynn_ok, I will submit a patch to deprecate it at last.05:01
elynn_ok, let me modify my patches and leave monkey to last patch.05:02
openstackgerritMerged openstack/senlin: Ensure that jsonutils.%(fun)s must be used instead of json.%(fun)s  https://review.openstack.org/26264605:09
Qimingok05:10
openstackgerritQiming Teng proposed openstack/senlin: Move app controller into dedicated module  https://review.openstack.org/26886905:23
openstackgerritQiming Teng proposed openstack/senlin: Add version controller for V1 API  https://review.openstack.org/26887405:54
openstackgerritMerged openstack/senlin: Move app controller into dedicated module  https://review.openstack.org/26886906:01
openstackgerritQiming Teng proposed openstack/senlin: Add version controller for V1 API  https://review.openstack.org/26887406:40
openstackgerritYanyan Hu proposed openstack/senlin: Fix a bug in profile base module  https://review.openstack.org/26889506:40
openstackgerritYanyan Hu proposed openstack/senlin: Fix a bug in profile base module  https://review.openstack.org/26889506:44
*** Qiming has quit IRC07:00
*** Qiming_ has joined #senlin07:00
openstackgerritMerged openstack/senlin: Fix a bug in profile base module  https://review.openstack.org/26889507:01
openstackgerritMerged openstack/senlin: Add version controller for V1 API  https://review.openstack.org/26887407:14
openstackgerritLiuqing Jing proposed openstack/python-senlinclient: The param endpoint in doc string is deprecated  https://review.openstack.org/26890307:29
*** elynn_ has quit IRC07:35
*** Qiming_ has quit IRC07:36
*** elynn_ has joined #senlin07:37
*** Qiming has joined #senlin07:42
openstackgerritQiming Teng proposed openstack/python-senlinclient: Unit test for v1 client  https://review.openstack.org/26891208:03
openstackgerritQiming Teng proposed openstack/senlin: Remove purge_deleted from senlin-manage  https://review.openstack.org/26891508:08
openstackgerritQiming Teng proposed openstack/senlin: Add 'cooldown' for scaling policy  https://review.openstack.org/26891808:15
lixinhuiQiming08:48
lixinhuithere?08:48
Qimingyes08:48
lixinhuiThe action in node_check should be check real status of node and set the DB?08:48
Qimingyes08:48
lixinhuiokay08:49
Qimingit will be left to users whether they want a recovery or not08:49
lixinhuiokay, once user call cluster-recover, we will help to recover all ndoes of status "ERROR"08:50
lixinhuiright?08:50
Qimingyes08:50
lixinhuiokay, thanks.08:50
Qiminghi, cores08:57
Qimingplease help review https://review.openstack.org/#/c/268912 ?08:57
Qimingneed it to fix get_node with details08:58
Qimingthx08:58
Yanyanhugot it08:58
elynn_+2 ;)08:59
Yanyanhuthis is a patch in client side?08:59
Qimingyip08:59
Yanyanhuok09:00
xuhaiweijust reviewing it09:00
Qiming:)09:03
openstackgerritMerged openstack/python-senlinclient: Unit test for v1 client  https://review.openstack.org/26891209:04
Yanyanhuhi, Qiming, just registered a bp for reworking action data transmission logic, e.g. the usage of action.data, action.input and action.output09:13
Yanyanhuhttps://blueprints.launchpad.net/senlin/+spec/rework-data-transmission-in-action-execution09:13
Yanyanhustill not very clear about some places, e.g. how to treat those builtin policies, considering them as part of action execution or treat them as customized policies09:15
openstackgerritQiming Teng proposed openstack/python-senlinclient: Add back show_details for node-show  https://review.openstack.org/26893809:16
Qimingsigh ...09:16
Yanyanhuthis is not a emergent work, just think the current action data transmission logic is still confusing when I worked on bp of policy check reworking09:17
Qimingwe won't get a chance to land everything before mitaka-309:17
Yanyanhuhmm, we don't have to finish this job in Mitaka cycle I think09:17
Qimingsince we are getting ride of priroity too early09:17
Qimingpolicy checking becomes a big problem now09:18
Qimingeven if we rework all cluster actions and node actions to do hard-coded policy checking, as you are experimenting with LB policy09:18
Qimingwe will still need to refactor the cluster_actions module to make it clear09:19
Yanyanhuyes, do need some thinking about this issue I think09:19
Qimingwith more and more checkings being added to the module, the code is becoming very complex09:19
Yanyanhuright, maybe we shouldn't just hard code those builtin check to address current issues09:20
Qimingone way of simplifying this would be to add subclasses of cluster actions09:20
Qimingeach action is split into an individual class09:20
Qimingthen we focus on each action, making sure each action is doing exactly what it should do, including the policy checks and action logic itself09:20
Yanyanhuso you mean each action class has its own policy_check logic09:22
Qimingyes09:22
Qimingthat would be an action-centric view09:22
Qimingcurrently we are working on a policy-centric view09:22
Yanyanhulet me see09:22
Yanyanhuyes, that's the point09:22
Qimingi.e. for each policy, which action should be handled, how to handle it09:23
Qimingaction-centric view would be dirty, there would be a lot of duplication of code, but it is much much more easier to understand and maintain09:24
Yanyanhuyes, it is09:24
Qimingthis is the root difference between haiwei and me about handling deletion across regions09:24
Yanyanhujust worried there will be lots of duplicated code logic09:25
Qimingaction centric view would be difficult to document as well09:25
Qimingfor each action, we have to document which policy would be checked and how09:25
Yanyanhuright09:25
Yanyanhuneed to think about this issue thoroughly09:25
Qimingyes09:26
Qimingbefore doing that09:26
Yanyanhuunderstand09:26
Qiminglet's think about what we can accomplish in THIS WEEK09:26
Qimingcan we release something usable?09:26
Yanyanhuactually I'm now very hesitated to do the work about rework-built-in policy bp09:26
Yanyanhusorry, rework builtin policy check bp09:26
Qimingyes, it is very dirty09:27
Yanyanhuyes, it is09:27
Qiminga compromise would be this09:27
Qimingstick to the original design09:27
Qimingassign each "builtin" policy a hard-coded priority09:28
QimingLB policy is an exception, it needs to be checked at both ends09:28
Yanyanhuok, maybe this is an acceptable interim solution for the first release09:29
Qimingand we can make that an exception because 'priority' or 'precedence' is an implementation detail now09:29
Yanyanhuyes09:29
QimingI'm not that optimistic that we can rework all those actions for hard-coded policies09:30
QimingI'm even not sure that is the right thing09:30
YanyanhuI see.09:30
Qiminginside senlin-engine09:30
Qimingwe can have scaling-policy carry a built in priority of 100, placement policy 200, lb policy 30009:31
Yanyanhuok, let me try to use hard-code priority to address lb_policy's issue before this friday09:31
Yanyanhuthen we rethink out policy design09:31
Yanyanhubefore starting any further refactoring09:31
Qimingagreed09:31
Yanyanhuok, will work following this way09:32
QimingI was reading the cluster_actions module this afternoon, between occasional patches here and there09:33
Qimingthere is no clean way to get the things right quickly09:34
Yanyanhuunderstand09:34
Qimingmaybe we should spend some time together tomorrow, to work out a plan on this09:43
Yanyanhusure, that's also what I'm thinking09:44
Qiming10:00-11:30 ?09:45
Yanyanhuok09:45
Qimingwe can continue during lunch and coffee time if our brains can last that long09:45
Yanyanhusure :)09:45
*** Qiming has quit IRC10:04
*** Yanyanhu has quit IRC10:04
*** elynn__ has joined #senlin10:36
*** elynn_ has quit IRC10:36
*** elynn__ has quit IRC10:51
*** Qiming has joined #senlin11:30
*** dixiaoli has quit IRC13:16
*** elynn__ has joined #senlin13:35
*** gongysh has joined #senlin13:37
openstackgerritLiuqing Jing proposed openstack/senlin: Method's default argument shouldn't be mutable  https://review.openstack.org/26905313:49
*** Liuqing has quit IRC13:54
*** gongysh has quit IRC14:48
*** elynn__ has quit IRC14:53
*** Qiming has quit IRC15:09
*** elynn has joined #senlin15:53
*** elynn has quit IRC15:54
*** jdandrea has quit IRC16:12
*** zhenguo has joined #senlin16:48
-openstackstatus- NOTICE: Gerrit is restarting quickly as a workaround for performance degradation16:50
*** pratikma_ has joined #senlin16:54
*** pratikma_ has quit IRC17:57
*** pratikmallya has joined #senlin19:05
*** pratikmallya has quit IRC21:32
*** pratikmallya has joined #senlin21:54
*** Qiming has joined #senlin23:41
*** pratikmallya has quit IRC23:43
*** yuanying has quit IRC23:54
*** yuanying has joined #senlin23:56

Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!