| *** zhurong has joined #openstack-zaqar | 00:21 | |
| *** wanghao has quit IRC | 00:36 | |
| *** wanghao has joined #openstack-zaqar | 00:37 | |
| *** zhurong has quit IRC | 00:37 | |
| wanghao | flwang: seems broken the zaqaclient, but has fixed: https://review.openstack.org/#/c/539189 | 00:49 |
|---|---|---|
| flwang | wanghao: got, thanks | 00:51 |
| flwang | wanghao: have you posted your PTL self-nomiation? | 00:51 |
| wanghao | flwang: I'm writing it now, post it today. | 00:53 |
| flwang | cool | 00:53 |
| wanghao | : ) | 00:53 |
| *** wanghao_ has joined #openstack-zaqar | 01:00 | |
| *** wanghao has quit IRC | 01:03 | |
| *** rcernin has joined #openstack-zaqar | 01:09 | |
| wxy | flwang: wanghao_ yeah, message always contains checksum, even it's a empty string. | 01:10 |
| wxy | I think we need a new patch for backward compatibility although it has been fixed at client side. | 01:12 |
| wxy | ++ yangzhenyu | 01:13 |
| *** wanghao_ has quit IRC | 01:16 | |
| *** wanghao has joined #openstack-zaqar | 01:17 | |
| yangzhenyu | wxy, https://review.openstack.org/#/c/534131/ | 01:33 |
| yangzhenyu | wxy, this patch has process that, but not merge | 01:34 |
| yangzhenyu | flwang, wanghao ^ | 01:34 |
| yangzhenyu | wxy, MD5 will be returned to the body of the message, the client must also have the corresponding changes, and get the message MD5 value can be displayed | 01:37 |
| wxy | yangzhenyu: the problem is here: https://bugs.launchpad.net/python-zaqarclient/+bug/1746226 | 01:38 |
| openstack | Launchpad bug 1746226 in Python client library for Zaqar "Message init fails with TypeError" [Undecided,Fix released] | 01:38 |
| wxy | yangzhenyu: after the server patch merged, client doesn't work. I know your patch can fix it. But it means that the server patch is backward incompatibility | 01:39 |
| yangzhenyu | wxy, yes, but md5 code need to modify the corresponding client | 01:41 |
| yangzhenyu | wxy, This feature must need add a new porperty checksum. | 01:42 |
| yangzhenyu | wxy, What do you think should be done? | 01:43 |
| wxy | yangzhenyu: so for backward compatibility, if the old message has no checksum, we should return nothing, not something like checksum: "" | 01:43 |
| yangzhenyu | wxy, | 01:45 |
| yangzhenyu | But this also can not solve the bug in front of it. | 01:45 |
| wanghao | do you guys think we should introduce the MV to solve this kind of issues? | 01:46 |
| wanghao | just discuss | 01:46 |
| wxy | yangzhenyu: oh YES. My fault here. | 01:46 |
| wanghao | from my view, if we add some new porperty in api, we should use MV to keep backcompatibility. | 01:47 |
| wxy | wanghao: that's what I want to say next. | 01:47 |
| wanghao | yes | 01:48 |
| wanghao | then may we don't to handle this issue like md5 patch. | 01:48 |
| yangzhenyu | wanghao, What is MV? | 01:49 |
| wxy | yangzhenyu: Sorry, to solve it, we can do: 1. add micro version, 2. add a switch that to handle checksum feature open or not. | 01:49 |
| wxy | micro version | 01:49 |
| yangzhenyu | o | 01:49 |
| wanghao | yeah | 01:49 |
| wanghao | seems in Queens, we can choose the option 2 | 01:50 |
| wanghao | but I'd like to use MV in future. | 01:50 |
| wanghao | #me leave for a while | 01:50 |
| wxy | yangzhenyu: does it make sense now? | 01:53 |
| yangzhenyu | wxy, add a config to handle checksum feature open or not? | 01:56 |
| wxy | yangzhenyu: A new release note could be : A new config option ``enabled_checksum`` is added. It's ``False`` by default in Queens to keep backward compatibility. Please note we'll enable it by default in Rocky and this option will be removed in S. | 01:57 |
| wxy | yangzhenyu: yeah. | 01:57 |
| *** wanghao has quit IRC | 01:57 | |
| wxy | A new release note could be : A new config option ``enabled_checksum`` is added. It's False by default in Queens to keep backward compatibility. Please note we'll enable it by default in Rocky and this option will be removed in S. | 01:58 |
| *** wanghao has joined #openstack-zaqar | 01:58 | |
| wxy | or a new parameter is OK as well. | 02:00 |
| yangzhenyu | wxy, wanghao But this change, the client will be wrong, because there is no checksum property. | 02:00 |
| yangzhenyu | I think just need merge https://review.openstack.org/#/c/534131/ | 02:01 |
| wxy | yangzhenyu: Then for some consumers who don't use zaqarclient, they need to change their client as well. | 02:02 |
| wxy | yangzhenyu: such as some sdks in different language. | 02:03 |
| yangzhenyu | ok, I will add it. | 02:05 |
| wxy | yangzhenyu: or a new parameter. like GET /messages?include_checksum=False or True. If no include_checksum, default it False in Q, True in R. | 02:07 |
| wxy | How do you like? | 02:07 |
| yangzhenyu | yes I think it's good | 02:08 |
| yangzhenyu | Thanks | 02:08 |
| *** ramishra has joined #openstack-zaqar | 02:09 | |
| ramishra | yangzhenyu, flwang1: hi | 02:11 |
| yangzhenyu | ramishra, hi | 02:11 |
| ramishra | We probably have to revert https://review.openstack.org/#/c/533863/ to unblock heat gate | 02:11 |
| ramishra | or we need a new zaqarclient release | 02:12 |
| ramishra | heat used to install zaqarclient from git earlier, but now after moving our tests to tempest-plugin we don't | 02:13 |
| wxy | revert seems better. We've just talked about the backward compatibility. I think we can fix it in the next PS. | 02:14 |
| yangzhenyu | ramishra, https://review.openstack.org/#/c/539189/1 | 02:14 |
| ramishra | yangzhenyu: yes, but that would only work once there is a client release donw | 02:14 |
| wxy | wanghao: here? | 02:15 |
| *** harlowja has quit IRC | 02:15 | |
| wanghao | here | 02:15 |
| wxy | flwang: ++ | 02:15 |
| openstackgerrit | Rabi Mishra proposed openstack/zaqar master: Revert "Support md5 of message body" https://review.openstack.org/539397 | 02:15 |
| ramishra | I've pushed a revert, you can decide to revert or make a client release asap. It's upto you | 02:16 |
| ramishra | heat is blocked till that time:( | 02:16 |
| wanghao | wxy: I agree to revert it. | 02:17 |
| flwang | ramishra: is there anyway to create a smoke test gate in zaqar for heat? | 02:17 |
| flwang | just like the tripleo one? | 02:17 |
| ramishra | flwang: it should be easy with zuulv3, just add a heat job in the pipeline? | 02:19 |
| ramishra | I can propose something later | 02:19 |
| flwang | ramishra: it would be great | 02:22 |
| yangzhenyu | flwang, but here has a client code. https://review.openstack.org/#/c/534131/ | 02:23 |
| yangzhenyu | I will change that md5 code, add a switch. | 02:24 |
| flwang | yangzhenyu: sorry? why do you say 'but'? | 02:27 |
| openstackgerrit | Rabi Mishra proposed openstack/zaqar master: Add heat job to zaqar check pipeline https://review.openstack.org/539402 | 02:30 |
| yangzhenyu | flwang, I mean this patch is a new implement for md5 of zaqarclient. | 02:31 |
| openstackgerrit | yangzhenyu proposed openstack/zaqar master: [WIP]Support md5 of message body https://review.openstack.org/539403 | 02:34 |
| flwang | yangzhenyu: i see | 02:35 |
| *** rcernin has quit IRC | 02:35 | |
| flwang | wanghao: wxy: can you approve this https://review.openstack.org/#/c/534131/4 ? | 02:40 |
| flwang | wanghao: and could you please release a new version for zaqar client? | 02:40 |
| wanghao | flwang: we just revert the md5 patch | 02:41 |
| wanghao | flwang: do we need to release a new version after revert? | 02:42 |
| flwang | sorry, i'm confused | 02:42 |
| flwang | i think we're reverting the client patch, right? | 02:42 |
| flwang | not the server patch | 02:42 |
| flwang | if we revert the client patch, we do still need yangzhenyu's client patch and a new release, no? | 02:43 |
| yangzhenyu | I think the easiest way now is to merge client and server at the same time. | 02:45 |
| yangzhenyu | Or I add a switch for md5. | 02:45 |
| flwang | what's the server change we need now? | 02:46 |
| yangzhenyu | Add a new parameter. like GET /messages?include_checksum=False or True. If no include_checksum, default it False in Q, True in R. | 02:47 |
| yangzhenyu | But I think the claim api also need it. | 02:47 |
| yangzhenyu | The md5 can be returned for claim, get, list operation. | 02:48 |
| *** wanghao__ has joined #openstack-zaqar | 02:49 | |
| yangzhenyu | flwang, Do you think we need to do this? | 02:49 |
| flwang | TBH, i don't like the extra parameter | 02:50 |
| flwang | i think it's useless | 02:51 |
| flwang | message in zaqar is a json/dict, it could contain anything | 02:51 |
| flwang | including the checksum | 02:52 |
| flwang | so the consumer can consume it if it's showing up or not | 02:52 |
| flwang | i don't think we should add a particular param to handle it | 02:52 |
| flwang | and it's not in the original design | 02:52 |
| yangzhenyu | wanghao, wxy flwang Anyway, we need unity of opinion. | 02:55 |
| wxy | yangzhenyu: flwang: then a new config option. | 03:03 |
| flwang | could we don't add the new parameter? | 03:12 |
| flwang | it's a bit ugly, IMHO | 03:12 |
| yangzhenyu | Then we only need to merge server and client code at same time. | 03:16 |
| yangzhenyu | Adding new parameters does increase the complexity of the code. Adding new parameters in Json should not have a significant impact on the user, affecting only the user's code writing compatibility is not good enough. | 03:21 |
| yangzhenyu | flwang, https://github.com/openstack/python-zaqarclient/blob/fd7a02588d05d816feb9392a8843ae3ae25bba90/zaqarclient/queues/v2/message.py#L60 | 03:28 |
| yangzhenyu | I think the compatibility of this implementation is not very good for adding new perporty. wxy wanghao | 03:28 |
| wxy | flwang: Of cause ,it's just one way. Not the only way. | 03:28 |
| wxy | yangzhenyu: ++ | 03:28 |
| wxy | flwang: yangzhenyu : So as I said before. A new config option could be OK. we just need to disable it in Q | 03:29 |
| wxy | once the client patch is merged and the we hold a while for consumers, we can open it certainly | 03:30 |
| wxy | For API changes, it's bother but we have to. Since we don't have micro version. | 03:32 |
| yangzhenyu | But even if we have a micro version, we can not add a it with a small change. | 03:34 |
| wxy | Actually I agree with flwang, message in zaqar is a json/dict, it could contain anything | 03:35 |
| yangzhenyu | yes | 03:35 |
| wxy | So on the other hand, it's a problem with our client. | 03:35 |
| yangzhenyu | https://github.com/openstack/python-zaqarclient/blob/fd7a02588d05d816feb9392a8843ae3ae25bba90/zaqarclient/queues/v2/message.py#L60 | 03:35 |
| wxy | Our client is not generic enough. | 03:35 |
| wanghao__ | yangzhenyu: for other api we should | 03:35 |
| wanghao__ | I remember we add a new porperty in message response body not message body | 03:37 |
| yangzhenyu | yes | 03:38 |
| wanghao__ | So I have different thought, not contain anything in there | 03:38 |
| wanghao__ | I mean you can contain anything in message body | 03:38 |
| wanghao__ | But when you add a new porperty in api response body. You should consider the back compatibility | 03:40 |
| wxy | wanghao: correct! | 03:40 |
| wxy | wanghao: do you think we should add MV for Zaqar. I'm a little hesitate. MV has its pro and con. We should be careful. | 03:43 |
| wanghao__ | We can discuss it in R | 03:43 |
| wanghao__ | You’re right we should be careful | 03:44 |
| *** flwang1 has quit IRC | 03:59 | |
| *** shu-mutou-AWAY has quit IRC | 04:15 | |
| *** rcernin has joined #openstack-zaqar | 04:50 | |
| *** harlowja has joined #openstack-zaqar | 04:51 | |
| *** rcernin has quit IRC | 04:51 | |
| *** rcernin has joined #openstack-zaqar | 04:52 | |
| yangzhenyu | wxy, wanghao If add a config item, do you have any sugge | 05:43 |
| yangzhenyu | any suggest for which section. | 05:44 |
| yangzhenyu | _GENERAL_OPTIONS? | 05:45 |
| wxy | a new section or _GENERAL_OPTIONS. Both are OK for me. | 06:10 |
| *** wanghao__ has quit IRC | 06:13 | |
| wanghao | _GENERAL_OPTIONS seems ok | 06:43 |
| *** harlowja has quit IRC | 07:01 | |
| openstackgerrit | Rabi Mishra proposed openstack/zaqar master: Add heat job to zaqar check pipeline https://review.openstack.org/539402 | 07:06 |
| *** rcernin has quit IRC | 07:08 | |
| *** wanghao_ has joined #openstack-zaqar | 07:13 | |
| *** wanghao has quit IRC | 07:13 | |
| *** wanghao has joined #openstack-zaqar | 07:14 | |
| *** wanghao_ has quit IRC | 07:18 | |
| openstackgerrit | yangzhenyu proposed openstack/zaqar master: [WIP]Support md5 of message body https://review.openstack.org/539403 | 07:30 |
| openstackgerrit | yangzhenyu proposed openstack/zaqar master: [WIP]Support md5 of message body https://review.openstack.org/539403 | 07:34 |
| *** pcaruana has joined #openstack-zaqar | 07:51 | |
| *** jtomasek has joined #openstack-zaqar | 08:05 | |
| *** rcernin has joined #openstack-zaqar | 08:16 | |
| *** tesseract has joined #openstack-zaqar | 08:20 | |
| openstackgerrit | OpenStack Proposal Bot proposed openstack/zaqar master: Imported Translations from Zanata https://review.openstack.org/538565 | 08:22 |
| openstackgerrit | yangzhenyu proposed openstack/zaqar master: [WIP]Support md5 of message body https://review.openstack.org/539403 | 08:25 |
| *** rcernin has quit IRC | 08:26 | |
| openstackgerrit | yangzhenyu proposed openstack/zaqar master: Support md5 of message body https://review.openstack.org/539403 | 08:34 |
| yangzhenyu | wanghao, wxy need review https://review.openstack.org/#/c/539403/ https://review.openstack.org/#/c/534131/ | 08:35 |
| wanghao | okay | 08:35 |
| wanghao | yangzhenyu: do we still need to check the self.driver.conf.enable_checksum? | 08:38 |
| yangzhenyu | wanghao, yes I think | 08:39 |
| yangzhenyu | the enable_checksum just use for unit test. | 08:39 |
| wanghao | I think we just add the option in generic option | 08:39 |
| wanghao | I see | 08:40 |
| yangzhenyu | yes the config option is generic option | 08:40 |
| wanghao | but it's a little werid to add the enable_checksum just for unit test. | 08:40 |
| yangzhenyu | https://review.openstack.org/#/c/539403/5/zaqar/common/configs.py | 08:40 |
| openstackgerrit | wangxiyuan proposed openstack/python-zaqarclient master: Support client of bp support-md5-of-body https://review.openstack.org/534131 | 08:41 |
| wxy | yangzhenyu: Looks good. but it conflicts with the revert patch. | 08:42 |
| yangzhenyu | wxy, ok i will process it. | 08:42 |
| wxy | yangzhenyu: cool. | 08:43 |
| wxy | yangzhenyu: can it still work if checksum is disabled in Zaqar? https://review.openstack.org/#/c/534131/5/zaqarclient/queues/v2/cli.py@252 | 08:45 |
| yangzhenyu | I think I must wait the revert one merge. | 08:45 |
| yangzhenyu | wxy, just display None | 08:46 |
| wxy | yangzhenyu: nice | 08:46 |
| *** zhurong_ has joined #openstack-zaqar | 08:46 | |
| yangzhenyu | wanghao, Can I get the config from wsgi layer? | 08:49 |
| wanghao | yangzhenyu: I mean if you just want to set the enable_checksum=True in UT, you can patch this config to True. | 08:54 |
| wanghao | no need to pass the argument from wsgi API | 08:54 |
| wanghao | let'm find a example | 08:54 |
| yangzhenyu | I do not pass it to wsgi. | 08:54 |
| yangzhenyu | wsgi layer not pass enable_checksum. | 08:55 |
| wanghao | ha sorry, I mean storage API | 08:55 |
| yangzhenyu | I know your mean, give me a example. | 08:55 |
| openstackgerrit | OpenStack Proposal Bot proposed openstack/zaqar-ui master: Imported Translations from Zanata https://review.openstack.org/539464 | 08:57 |
| wanghao | yangzhenyu: see this : test_send_confirm_notification | 09:01 |
| wanghao | just need self.conf.notification.require_confirmation = True | 09:01 |
| wanghao | like this | 09:02 |
| yangzhenyu | OK, It is good for this. | 09:04 |
| *** rcernin has joined #openstack-zaqar | 09:05 | |
| openstackgerrit | yangzhenyu proposed openstack/zaqar master: Support md5 of message body https://review.openstack.org/539403 | 09:10 |
| *** wanghao_ has joined #openstack-zaqar | 09:14 | |
| *** wanghao has quit IRC | 09:14 | |
| *** wanghao has joined #openstack-zaqar | 09:23 | |
| *** wanghao_ has quit IRC | 09:26 | |
| openstackgerrit | Merged openstack/zaqar master: Revert "Support md5 of message body" https://review.openstack.org/539397 | 09:29 |
| *** wanghao has quit IRC | 09:35 | |
| *** wanghao has joined #openstack-zaqar | 09:35 | |
| *** wanghao has quit IRC | 09:36 | |
| *** wanghao has joined #openstack-zaqar | 09:36 | |
| *** wanghao has quit IRC | 09:36 | |
| *** wanghao has joined #openstack-zaqar | 09:38 | |
| *** wanghao has quit IRC | 09:38 | |
| *** wanghao_ has joined #openstack-zaqar | 09:39 | |
| *** wanghao_ has quit IRC | 09:40 | |
| *** wanghao_ has joined #openstack-zaqar | 09:40 | |
| *** wanghao_ has quit IRC | 09:40 | |
| *** wanghao_ has joined #openstack-zaqar | 09:41 | |
| *** wanghao_ has quit IRC | 09:41 | |
| *** wanghao_ has joined #openstack-zaqar | 09:41 | |
| *** wanghao_ has quit IRC | 09:42 | |
| *** wanghao_ has joined #openstack-zaqar | 09:42 | |
| *** wanghao_ has quit IRC | 09:43 | |
| *** wanghao has joined #openstack-zaqar | 09:43 | |
| *** wanghao has quit IRC | 09:44 | |
| *** wanghao has joined #openstack-zaqar | 09:44 | |
| *** zhurong_ has quit IRC | 10:48 | |
| openstackgerrit | yangzhenyu proposed openstack/zaqar master: Support md5 of message body https://review.openstack.org/539403 | 11:04 |
| openstackgerrit | yangzhenyu proposed openstack/zaqar master: Support md5 of message body https://review.openstack.org/539403 | 11:20 |
| *** wanghao_ has joined #openstack-zaqar | 11:21 | |
| *** wanghao has quit IRC | 11:24 | |
| openstackgerrit | yangzhenyu proposed openstack/zaqar master: Support md5 of message body https://review.openstack.org/539403 | 12:47 |
| *** rcernin has quit IRC | 12:58 | |
| *** pcaruana has quit IRC | 14:05 | |
| *** pcaruana has joined #openstack-zaqar | 14:21 | |
| *** jtomasek_ has joined #openstack-zaqar | 14:22 | |
| *** jtomasek has quit IRC | 14:24 | |
| *** pcaruana has quit IRC | 14:48 | |
| *** wanghao_ has quit IRC | 14:50 | |
| *** pcaruana has joined #openstack-zaqar | 15:14 | |
| *** pcaruana has quit IRC | 16:39 | |
| *** harlowja has joined #openstack-zaqar | 17:02 | |
| *** ramishra has quit IRC | 17:03 | |
| *** flwang1 has joined #openstack-zaqar | 17:37 | |
| *** tesseract has quit IRC | 17:45 | |
| *** flwang1 has quit IRC | 18:37 | |
| *** harlowja has quit IRC | 18:52 | |
| *** harlowja has joined #openstack-zaqar | 19:37 | |
| *** flwang1 has joined #openstack-zaqar | 20:38 | |
| *** rcernin has joined #openstack-zaqar | 23:28 | |
| *** rcernin has quit IRC | 23:50 | |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!