| openstackgerrit | xu-haiwei proposed stackforge/senlin: Handle exceptions in keystone_v3 driver https://review.openstack.org/213569 | 00:32 |
|---|---|---|
| *** zhenguo has joined #senlin | 00:43 | |
| *** Qiming has joined #senlin | 01:04 | |
| openstackgerrit | Merged stackforge/senlin: Use wait_for_delete to wait for nova server deletion https://review.openstack.org/214448 | 01:10 |
| *** mathspanda has joined #senlin | 01:12 | |
| *** Yanyanhu has joined #senlin | 01:16 | |
| xuhaiwei | Qiming, morning | 01:17 |
| xuhaiwei | have a question about webhook | 01:18 |
| openstackgerrit | Merged stackforge/senlin: Check cluster size constraint before doing node join/leave https://review.openstack.org/215475 | 01:18 |
| Qiming | yes | 01:18 |
| xuhaiwei | currently the webhook url is not stored in the database, is that ok? | 01:18 |
| xuhaiwei | from webhook-show , we can't refer to the url | 01:19 |
| Qiming | em, that was by design | 01:20 |
| Qiming | it was meant to be shown just once, for security's reason | 01:20 |
| *** Yanyanhu has quit IRC | 01:21 | |
| Qiming | the user is supposed to save it safely, especially the 'key' part | 01:21 |
| xuhaiwei | ok | 01:21 |
| Qiming | maybe it is a bad design | 01:21 |
| *** Yanyanhu has joined #senlin | 01:21 | |
| Qiming | we can revisit it if it turns out to be very inconvenient | 01:21 |
| xuhaiwei | and another question, from webhook-show we can't see the webhook's status either | 01:22 |
| xuhaiwei | even webhook is not correctly created, it is still in the webhook list | 01:23 |
| Qiming | webhook doesn't have a status field? | 01:23 |
| xuhaiwei | no | 01:23 |
| Qiming | we are not maintaining status for webhook | 01:23 |
| Qiming | we even don't have an update API for webhook | 01:24 |
| xuhaiwei | I mean use may don't know whether webhook is correctly created | 01:24 |
| Yanyanhu | if error happened during webhook creation, the webhook will not be stored in DB and you can't see it in webhook list | 01:24 |
| Yanyanhu | since webhook's creation is not done in action progress :) | 01:25 |
| Qiming | it is like a profile | 01:25 |
| Yanyanhu | just like profile or policy | 01:25 |
| xuhaiwei | no, Yanyanhu, it's in the list | 01:25 |
| Yanyanhu | yep | 01:25 |
| Qiming | if it is shown there, it must have been created | 01:25 |
| Yanyanhu | oh? | 01:25 |
| Yanyanhu | let me have a look | 01:25 |
| Qiming | okay, sounds like a defect | 01:25 |
| Yanyanhu | hmm, if so, it is | 01:25 |
| xuhaiwei | I mean if the webhook url is not created, the webhook is still in the list, without a 'ERROR' status | 01:26 |
| xuhaiwei | because it's not an action, we can't check it in action-list | 01:26 |
| Yanyanhu | http://git.openstack.org/cgit/stackforge/senlin/tree/senlin/engine/service.py#n1292 | 01:27 |
| openstackgerrit | Merged stackforge/senlin: Handle exceptions in keystone_v3 driver https://review.openstack.org/213569 | 01:27 |
| Yanyanhu | since the webhook_url generatoin is after webhook.store | 01:27 |
| xuhaiwei | yes | 01:27 |
| xuhaiwei | url is not in database | 01:27 |
| Yanyanhu | hmm, I think maybe we should add a return value check or exceptoin handling in line 1293 | 01:27 |
| Yanyanhu | to help decide whether webhook_url is generated correctly. If not, we remove the webhook just created from DB | 01:28 |
| Yanyanhu | and return error msg | 01:28 |
| xuhaiwei | because it is already stored in db in line 1292, add exception handing in line 1293 does not help I think | 01:29 |
| Qiming | can we postpone the store() call? | 01:30 |
| Yanyanhu | you can remove it from DB if exception happened in 1293 | 01:30 |
| xuhaiwei | check url first and then store in db? | 01:30 |
| Yanyanhu | Qiming, I remeber there is a dependency here, so the stored is put before webhook.generate_url | 01:31 |
| xuhaiwei | though url is not stored | 01:31 |
| Yanyanhu | store operation | 01:31 |
| Qiming | my understanding is that you need the UUID of the webhook | 01:31 |
| Yanyanhu | oh, right | 01:31 |
| Qiming | but you can generate manually before handling it to DB | 01:31 |
| Yanyanhu | self.id is needed | 01:31 |
| Yanyanhu | umm, so we can generate the id at service module and use it to initialize the webhook obj | 01:32 |
| Yanyanhu | if so, we may need to change the logic of Webhook.store method | 01:33 |
| Qiming | why? | 01:33 |
| Yanyanhu | since current implementation doesn't include id field when invoke db_api.webhook_create | 01:34 |
| Yanyanhu | the id will be generated in DB side | 01:35 |
| Yanyanhu | I think put store after generate_url is the correct way | 01:35 |
| Qiming | anyway, we don't have webhook update API right? | 01:35 |
| Yanyanhu | right | 01:35 |
| Yanyanhu | a easy change I think | 01:36 |
| xuhaiwei | because there is no status field of webhook, should return an exception to user when the creation failed | 01:37 |
| Qiming | yes | 01:38 |
| Yanyanhu | I think an error msg is ok | 01:38 |
| Yanyanhu | just put the error info rather than webhook dictionary into the return value | 01:38 |
| xuhaiwei | Yanyanhu, I am afraid that doesn't work well | 01:39 |
| Yanyanhu | oh? | 01:40 |
| xuhaiwei | return False instead of result? | 01:40 |
| Yanyanhu | no | 01:41 |
| Yanyanhu | ho, let me see | 01:41 |
| Yanyanhu | so the resource create API should always return a dictionary in the following format? {'webhook': {...}} | 01:43 |
| Yanyanhu | for cluster, should be {'cluster': {...}} | 01:43 |
| Yanyanhu | if no exception happened during creation? | 01:43 |
| Yanyanhu | cluster is not a good example here | 01:43 |
| Yanyanhu | similar resp for policy, profile | 01:44 |
| xuhaiwei | seem so | 01:44 |
| Yanyanhu | if so, a error msg string in return value is not a correct response based on restful standard? | 01:45 |
| xuhaiwei | you mean we store the error msg like result['msg'], and return it back? | 01:46 |
| Yanyanhu | yes | 01:46 |
| xuhaiwei | that is not as good as an exception I think | 01:46 |
| Qiming | if we unfortunately fails a webhook creation, we will need to raise an exception just like BadRequest | 01:48 |
| Yanyanhu | yes | 01:48 |
| Qiming | but it may have to be an 500 internal server error | 01:48 |
| Qiming | it would be our fault to fail that creation, right? | 01:49 |
| Yanyanhu | Qiming, I think for exception happened in generate_url, that's true | 01:49 |
| xuhaiwei | yes, i think | 01:49 |
| Qiming | Yanyanhu, if URL is not correctly generated, the webhook will be useless | 01:49 |
| Yanyanhu | we already have some exceptoin handling for those cases like target object not found | 01:49 |
| Yanyanhu | yes | 01:50 |
| xuhaiwei | but, sometime senlin service is not started will also cause this error | 01:50 |
| Qiming | target object not found is a client side error | 01:50 |
| Qiming | user is naming a non-existent entity for webhook | 01:50 |
| Yanyanhu | hmm, you're right | 01:50 |
| Qiming | but if generate_url fails, it is server's fault, don't blame the requesters | 01:50 |
| Yanyanhu | currently, we also treat those exceptions as badrequest | 01:51 |
| xuhaiwei | yap | 01:51 |
| Yanyanhu | yep | 01:51 |
| Qiming | so, fix it, man! | 01:51 |
| xuhaiwei | ok, i will take it | 01:51 |
| Yanyanhu | great :) | 01:51 |
| Qiming | \o/ | 01:51 |
| *** jdandrea has quit IRC | 02:27 | |
| *** lkarm has joined #senlin | 02:50 | |
| *** lkarm has quit IRC | 02:54 | |
| openstackgerrit | xu-haiwei proposed stackforge/senlin: Fail webhook creation if url is not correctly created https://review.openstack.org/216508 | 03:12 |
| *** jruano has quit IRC | 03:13 | |
| *** xuhaiwei has quit IRC | 04:06 | |
| *** xuhaiwei has joined #senlin | 05:07 | |
| openstackgerrit | Yanyan Hu proposed stackforge/senlin: Remove context usage from driver layer https://review.openstack.org/216529 | 05:12 |
| openstackgerrit | xu-haiwei proposed stackforge/python-senlinclient: Delete deleted_time and url option from webhook-show https://review.openstack.org/216535 | 05:42 |
| *** zhenguo has quit IRC | 05:53 | |
| *** zhenguo has joined #senlin | 06:09 | |
| openstackgerrit | Zhenguo Niu proposed stackforge/senlin-dashboard: Bump pbr version to match global requirement https://review.openstack.org/216544 | 06:16 |
| *** xuhaiwei has quit IRC | 06:29 | |
| *** xuhaiwei has joined #senlin | 06:29 | |
| openstackgerrit | xu-haiwei proposed stackforge/python-senlinclient: Add deleted_time to webhook_list https://review.openstack.org/216553 | 06:32 |
| openstackgerrit | Qiming Teng proposed stackforge/senlin: API layer support for triggers https://review.openstack.org/216556 | 06:49 |
| openstackgerrit | Qiming Teng proposed stackforge/senlin: API layer support for triggers https://review.openstack.org/216556 | 06:57 |
| openstackgerrit | Qiming Teng proposed stackforge/senlin: API layer support for triggers https://review.openstack.org/216556 | 06:58 |
| openstackgerrit | Merged stackforge/senlin-dashboard: Bump pbr version to match global requirement https://review.openstack.org/216544 | 06:59 |
| *** Qiming has quit IRC | 07:01 | |
| *** Qiming has joined #senlin | 07:06 | |
| openstackgerrit | OpenStack Proposal Bot proposed stackforge/senlin: Updated from global requirements https://review.openstack.org/216574 | 07:12 |
| *** lkarm has joined #senlin | 07:22 | |
| *** lkarm has quit IRC | 07:27 | |
| *** mathspanda has quit IRC | 07:49 | |
| openstackgerrit | Yanyan Hu proposed stackforge/senlin: Remove context usage from driver layer https://review.openstack.org/216529 | 08:05 |
| Qiming | https://review.openstack.org/#/c/214038/ | 08:05 |
| Qiming | this change was a complete mess | 08:05 |
| Qiming | sigh ... | 08:05 |
| Yanyanhu | oh? | 08:06 |
| Qiming | the adding of service credentials in engine.service is like a virus | 08:07 |
| Qiming | it is polluting the db, and the webhook flow | 08:07 |
| Yanyanhu | you mean this operation should be done in webhook module? | 08:07 |
| Yanyanhu | if so, maybe we should add something like get_default_credential in webhook.py | 08:09 |
| Qiming | http://git.openstack.org/cgit/stackforge/senlin/tree/senlin/engine/service.py#n1264 | 08:09 |
| Qiming | adding this line is producing a lot of garbage in credential | 08:09 |
| Qiming | we cannot differentiate what was provided by users, what was defaults from the engine | 08:10 |
| Qiming | the worst thing is that it copied 'project_name' from service_credetials into the dict | 08:10 |
| Qiming | and project_domain_name | 08:10 |
| Yanyanhu | but I think this line will work only when user doesn't provide their own credential? | 08:10 |
| Qiming | it won't | 08:11 |
| Qiming | it won't work under all situation | 08:11 |
| Qiming | because the project_name is set to 'service' by this line | 08:11 |
| Yanyanhu | you mean? | 08:11 |
| Qiming | and we have 'trusts' in the same dict | 08:12 |
| Yanyanhu | ah, I see. So actually we should do something like patchset 3 | 08:13 |
| Qiming | line 1268 is wrong too | 08:13 |
| Qiming | maybe I'm not understanding it correctly | 08:14 |
| Yanyanhu | hmm, actually I found set trusts to a string of trust_id also worked, this is weird | 08:14 |
| Yanyanhu | just as you said we change it to list in a previous patch | 08:15 |
| Qiming | at least it should be cred.cred['openstack']['trust'] | 08:15 |
| Yanyanhu | umm, that's true. But it did passed the pratical test... | 08:16 |
| Qiming | ... | 08:16 |
| Yanyanhu | hmm, actually I ran the pratical test using patchset 3... | 08:17 |
| openstackgerrit | Merged stackforge/senlin: Updated from global requirements https://review.openstack.org/216574 | 08:27 |
| Yanyanhu | hi, Qiming, so about the error in webhook_create, should we implement the credential generation logic in webhook module and refer it in service module? | 08:41 |
| Qiming | trying to fix it now | 08:44 |
| Yanyanhu | ok | 08:44 |
| Qiming | the basic idea is save just what user provided in an encrypted form | 08:44 |
| Yanyanhu | the senlin service related part will be added in trigger progress? | 08:45 |
| Qiming | construct a dict for token retrieval in webhook middleware when it is triggered | 08:45 |
| Yanyanhu | ok | 08:45 |
| Yanyanhu | much better | 08:45 |
| Qiming | override the dict content with whatever decrypted from the user provided "credential" | 08:45 |
| Yanyanhu | ok, understand | 08:46 |
| Qiming | there is a chicken-egg problem though | 08:47 |
| Yanyanhu | ah, I suddenly understand... | 08:48 |
| Yanyanhu | I think I misunderstood your comment in patchset3... https://review.openstack.org/#/c/214038/3/senlin/engine/service.py | 08:48 |
| Yanyanhu | in line1265 | 08:48 |
| Yanyanhu | about this 'treat the 'credential' field as a customization to the "default connection params"' | 08:49 |
| Qiming | okay, I meant a dict.update() call | 08:50 |
| Yanyanhu | yes | 08:50 |
| openstackgerrit | Yanyan Hu proposed stackforge/senlin: Remove context usage from driver layer https://review.openstack.org/216529 | 08:54 |
| xuhaiwei | what shall we do if the user 'credential' contains something useless? | 08:57 |
| *** Yanyanhu_ has joined #senlin | 08:58 | |
| xuhaiwei | just store it in the db? | 08:59 |
| *** Yanyanhu has quit IRC | 09:00 | |
| xuhaiwei | since user 'credential' is not essential, don't know why we need to check whether 'credential' is provided | 09:04 |
| *** Yanyanhu_ has quit IRC | 09:04 | |
| lawrancejing | HI Qiming, i'm pleasure to contribute to senlin-dashboard, could i join the senlin-dashboard team? | 09:04 |
| Qiming | sure | 09:04 |
| Qiming | thank you so much for helping us out there | 09:04 |
| *** Yanyanhu_ has joined #senlin | 09:05 | |
| Qiming | xuhaiwei, credential is not mandatory | 09:05 |
| xuhaiwei | yes, so I really dont understand why we check it is provided, http://git.openstack.org/cgit/stackforge/senlin/tree/senlin/engine/service.py#n1263 | 09:06 |
| xuhaiwei | since if user provide something useless to 'credential' , this check will be skipped | 09:07 |
| Qiming | if it is not provided, we will use the "default credential" | 09:07 |
| lawrancejing | OK cheers | 09:07 |
| Qiming | by "credential", it means who will be triggering the webhook in future | 09:07 |
| Qiming | lawrancejing, feel free to dance and sing there, ;) | 09:08 |
| lawrancejing | :😃 sure | 09:08 |
| lawrancejing | and thanks | 09:08 |
| zhenguo | welcome lawrancejing | 09:11 |
| xuhaiwei | Qiming, by the 'default credential', it is the 'trust_id', right? but if user provides their 'credential', what should it be? user_name or project_name?? it seems they are already in the 'kwargs' | 09:13 |
| Qiming | they can provided their own username and/or password and/or whatever they would like | 09:14 |
| Qiming | by kwargs, you mean the 1280 line? | 09:14 |
| xuhaiwei | yes | 09:14 |
| Qiming | those are the object owner/creator | 09:15 |
| Qiming | it only tells you who created this webhook, to which project the webhook resides in | 09:16 |
| xuhaiwei | that makes sense to me | 09:16 |
| Qiming | if "John" created a webhook, and the webhook can be triggered only by "John", that will be a silly design, right? | 09:17 |
| xuhaiwei | yes | 09:18 |
| Yanyanhu_ | hi, lawrancejing, welcome to senlin, just missed your message for network disconnection :) | 09:19 |
| Qiming | xuhaiwei, so the design is like this | 09:19 |
| Qiming | Alice can create a webhook for objects she owns | 09:20 |
| lawrancejing | Yanyanhu_ thanks | 09:20 |
| Qiming | Alice can create webhooks for whatever objects, if she is admin | 09:20 |
| xuhaiwei | Qiming, got it, I think | 09:20 |
| xuhaiwei | lawrancejing, welcome to senlin | 09:21 |
| Qiming | When webhook is triggered, we will find out "who are you", "are you admin", "do you own the object" ... | 09:21 |
| lawrancejing | thanks all guys：） | 09:21 |
| xuhaiwei | the webhooks are usually not triggered from the command line, right? | 09:22 |
| Qiming | yes | 09:24 |
| Qiming | maybe the common use case would be an admin creates them and shares them to cluster owners | 09:25 |
| xuhaiwei | will leave now, see you tonight | 09:31 |
| Qiming | see you | 09:31 |
| Yanyanhu_ | see U | 09:31 |
| *** lawrance_ has joined #senlin | 09:53 | |
| *** lawrance_ has quit IRC | 09:56 | |
| *** lawrance_ has joined #senlin | 09:56 | |
| *** lawrancejing has left #senlin | 10:01 | |
| *** lawrance_ has quit IRC | 10:03 | |
| *** lawrance_ has joined #senlin | 10:03 | |
| *** lawrance_ has left #senlin | 10:03 | |
| *** lawrance_ has joined #senlin | 10:07 | |
| *** lawrance_ has quit IRC | 10:22 | |
| *** jruano has joined #senlin | 10:44 | |
| *** Yanyanhu_ has quit IRC | 10:47 | |
| *** Qiming has quit IRC | 10:56 | |
| openstackgerrit | Merged stackforge/senlin: Add fake nova_v2 driver for functional test https://review.openstack.org/214090 | 11:37 |
| *** zhenguo has quit IRC | 11:55 | |
| *** Qiming has joined #senlin | 11:57 | |
| *** Qiming has quit IRC | 12:06 | |
| *** Qiming has joined #senlin | 12:06 | |
| *** lkarm has joined #senlin | 12:08 | |
| jruano | hi qiming... do you have time now to meet with matt and i re: tosca policies? | 12:10 |
| Qiming | hi, yes | 12:12 |
| Qiming | just back home | 12:12 |
| jruano | no worries. we're on the callin | 12:13 |
| *** Qiming has quit IRC | 12:17 | |
| *** Qiming has joined #senlin | 12:17 | |
| *** haiwei has joined #senlin | 12:31 | |
| haiwei | hi | 12:31 |
| *** haiwei has quit IRC | 12:42 | |
| *** yanyanhu has joined #senlin | 12:55 | |
| Qiming | previous meeting overrun again, sorry | 13:00 |
| yanyanhu | it ended | 13:01 |
| *** zhenguo has joined #senlin | 13:16 | |
| Qiming | admin__, online? | 13:21 |
| admin__ | yes | 13:21 |
| Qiming | openstack-meeting | 13:21 |
| yanyanhu | oh, admin__ is xinhui? | 13:22 |
| admin__ | Yes, Yanyanhu | 13:22 |
| yanyanhu | :) | 13:23 |
| *** haiwei_ has joined #senlin | 13:49 | |
| *** yanyanhu has quit IRC | 13:49 | |
| Qiming | seems yanyanhu dropped, haiwei | 13:50 |
| haiwei_ | ok | 13:50 |
| haiwei_ | will say to him tomorrow | 13:51 |
| Qiming | ok | 13:51 |
| Qiming | too late for you | 13:51 |
| haiwei_ | yap, but you often stay late | 13:51 |
| Qiming | I'm growing old, haha | 13:52 |
| haiwei_ | the trip you had last week seems great | 13:53 |
| Qiming | yes, it was cool | 13:54 |
| Qiming | it feels good when people understand what you are talking about, :) | 13:54 |
| haiwei_ | you mean the local people? | 13:55 |
| haiwei_ | ok, will go to bed, see you | 13:56 |
| *** jruano has quit IRC | 13:57 | |
| Qiming | see you | 13:58 |
| openstackgerrit | Merged stackforge/python-senlinclient: Add deleted_time to webhook_list https://review.openstack.org/216553 | 13:58 |
| *** haiwei_ has quit IRC | 14:00 | |
| openstackgerrit | Merged stackforge/senlin: Remove context usage from driver layer https://review.openstack.org/216529 | 14:11 |
| *** lawrancejing has joined #senlin | 14:42 | |
| *** lawrance_ has joined #senlin | 14:51 | |
| *** lawrancejing has left #senlin | 14:53 | |
| openstackgerrit | Qiming Teng proposed stackforge/senlin: Fix webhook triggering logic https://review.openstack.org/216751 | 15:02 |
| *** jruano has joined #senlin | 15:11 | |
| openstackgerrit | Qiming Teng proposed stackforge/senlin: API layer support for triggers https://review.openstack.org/216556 | 15:17 |
| openstackgerrit | Zhenguo Niu proposed stackforge/senlin-dashboard: Add cluster dashboard https://review.openstack.org/216762 | 15:19 |
| Qiming | heading to bed | 15:21 |
| Qiming | bbl | 15:21 |
| *** jdandrea has joined #senlin | 15:23 | |
| *** Qiming has quit IRC | 15:26 | |
| *** zhenguo has quit IRC | 15:36 | |
| *** lawrance_ has quit IRC | 15:39 | |
| *** jruano has quit IRC | 19:02 | |
| *** lkarm has quit IRC | 21:04 | |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!