| *** chandankumar has quit IRC | 00:28 | |
| *** apetrich has quit IRC | 03:14 | |
| *** irclogbot_1 has quit IRC | 04:57 | |
| *** jrist has quit IRC | 05:06 | |
| *** jrist has joined #openstack-mistral | 05:10 | |
| *** quiquell|off is now known as quiquell|rover | 06:29 | |
| *** apetrich has joined #openstack-mistral | 06:38 | |
| *** jtomasek has joined #openstack-mistral | 06:38 | |
| openstackgerrit | Adriano Petrich proposed openstack/mistral master: Return empty list as tuple https://review.openstack.org/637507 | 07:13 |
|---|---|---|
| apetrich | rakhmerov, I added the tests from the comments ^^ but that is failing | 07:16 |
| *** quiquell|rover is now known as quique|rover|r-- | 07:26 | |
| *** akovi has joined #openstack-mistral | 07:52 | |
| *** pgaxatte has joined #openstack-mistral | 08:14 | |
| *** shardy has joined #openstack-mistral | 08:15 | |
| *** shardy has quit IRC | 08:15 | |
| *** shardy has joined #openstack-mistral | 08:31 | |
| *** jtomasek has quit IRC | 08:36 | |
| *** quique|rover|r-- is now known as quiquell|rover | 08:46 | |
| d0ugal | apetrich: Yeah, it'll never pass :) | 08:49 |
| d0ugal | apetrich: the patch handles empty lists only, that is why it isn't a correct fix | 08:49 |
| apetrich | d0ugal, I know but I wanted the regression tests there | 08:57 |
| d0ugal | ah, cool | 08:58 |
| d0ugal | makes sense | 08:58 |
| * d0ugal is still waking up | 08:58 | |
| apetrich | I wonder if we should just remove that way of comparing | 08:59 |
| d0ugal | How? I'm not sure we can really | 08:59 |
| apetrich | because bool() would work for the empty lists | 09:00 |
| apetrich | oh. | 09:00 |
| d0ugal | it should also be deprecated really | 09:00 |
| apetrich | not remove just not use it | 09:00 |
| d0ugal | Right, that is the solution in tripleo anyway | 09:00 |
| d0ugal | Did you see the fix merged? | 09:00 |
| d0ugal | https://github.com/openstack/tripleo-common/commit/573f67df3d6216f62467eb526a15be3120f9a01a | 09:00 |
| apetrich | d0ugal, no I didn't | 09:02 |
| apetrich | d0ugal, nice. that means we are not on the critical issues | 09:03 |
| quiquell|rover | d0ugal: I am going to try to continue with the list fix | 09:03 |
| d0ugal | quiquell|rover: Great, I think rakhmerov is also looking into it. So make sure you don't duplicate effort :) | 09:03 |
| d0ugal | apetrich: Yeah, now it is just a gotcha | 09:04 |
| d0ugal | but one that wont come up often, I hope | 09:04 |
| quiquell|rover | d0ugal: Yep stuff is working now at tripleo | 09:04 |
| quiquell|rover | d0ugal: maybe is good to document that though | 09:04 |
| quiquell|rover | d0ugal: like "don't use = []" | 09:04 |
| apetrich | quiquell|rover, great work | 09:04 |
| d0ugal | Maybe, but I don't know where I would document it :) | 09:04 |
| d0ugal | I assume nobody would read the docs anyway lol | 09:04 |
| quiquell|rover | d0ugal: pragmatic guy you are | 09:05 |
| d0ugal | but maybe a mistral bug we can reference would be wise | 09:05 |
| quiquell|rover | Ok let's try to sort out the comments to have the review there in good shape | 09:05 |
| d0ugal | Sounds good, thanks! | 09:05 |
| quiquell|rover | d0ugal: so the review is has a legit test failure and we have to find a way to cover the cases | 09:12 |
| quiquell|rover | d0ugal: is that it ? | 09:13 |
| d0ugal | quiquell|rover: Yes, I believe so | 09:13 |
| d0ugal | Fix those tests and don't break any others :) | 09:13 |
| quiquell|rover | d0ugal: ack tdd !!! | 09:14 |
| quiquell|rover | d0ugal, apetrich: It's not quite clear when do we have to return a tuple and when a list | 09:19 |
| d0ugal | Agreed. I don't fully understand either. I think you might need rakhmerov to understand the logic | 09:19 |
| apetrich | +1 | 09:19 |
| quiquell|rover | d0ugal: looks complex stuff like, really understand yaql asumptions | 09:19 |
| quiquell|rover | apetrich, d0ugal: will wait to rakhmerov is this is ok | 09:20 |
| d0ugal | Yup, that is fine | 09:25 |
| rakhmerov | d0ugal: hi, what's up? | 09:28 |
| rakhmerov | I stepped out and just got back | 09:28 |
| rakhmerov | so missed what you were discussing | 09:28 |
| d0ugal | rakhmerov: Nothing much, quiquell|rover was just looking how to fix the YAQL issue correctly | 09:28 |
| rakhmerov | aah.. | 09:28 |
| d0ugal | but I think the conclusion was to let you handle it as you have more insight | 09:28 |
| rakhmerov | I spent a sleepless night thinking about it :) | 09:28 |
| quiquell|rover | rakhmerov: Yep is more than a quick fix | 09:28 |
| rakhmerov | so, I don't have a good fix in mind yet | 09:29 |
| d0ugal | It is tricky | 09:29 |
| quiquell|rover | btw why don't mistral just use tuples ? is the "str" issue ? | 09:29 |
| rakhmerov | and moreover, I don't think there's a way to satisfy both things, the 'str' function and this comparison | 09:29 |
| rakhmerov | quiquell|rover: str($.my_list) gives "(1,3,4,)" | 09:30 |
| rakhmerov | which is confusing | 09:30 |
| rakhmerov | and not only confusing but it was a regression made in Rocky | 09:30 |
| rakhmerov | before Rocky it would return "[1,2,3]" | 09:31 |
| quiquell|rover | maybe upstream fix a yaql ? or this is crazy ? | 09:31 |
| d0ugal | You could use the JSON function to create that result | 09:32 |
| rakhmerov | d0ugal: the question is in compatibility | 09:36 |
| d0ugal | Sure | 09:36 |
| rakhmerov | it used to work | 09:36 |
| d0ugal | rakhmerov: What changed in Rocky? | 09:36 |
| rakhmerov | we added this pre-processing | 09:37 |
| rakhmerov | of data context | 09:37 |
| rakhmerov | we did it because we couldn't have sets of dicts | 09:37 |
| d0ugal | Right | 09:37 |
| quiquell|rover | d0ugal: It could have change yaql rpm update | 09:37 |
| quiquell|rover | we were using python2-yaql-1.1.3-2.el7.noarch | 09:38 |
| d0ugal | rakhmerov: did you consider changing your dicts to frozendicts? | 09:39 |
| rakhmerov | quiquell|rover: fixing yaql is an option yes, but it'll take a lot of time | 09:40 |
| rakhmerov | d0ugal: maybe, I don't remember all the details of that now | 09:41 |
| quiquell|rover | rakhmerov: I can compare yaql versions between working and not working | 09:41 |
| quiquell|rover | rakhmerov: if this can help you guys | 09:41 |
| rakhmerov | it's the same | 09:41 |
| quiquell|rover | ack | 09:41 |
| quiquell|rover | :-/ | 09:41 |
| rakhmerov | yaql version is the same, it hasn't changed for 2-3 years | 09:41 |
| d0ugal | rakhmerov: np, just an idea. That is how I would normally do sets of dicts | 09:41 |
| rakhmerov | I guess no one really supports it | 09:41 |
| rakhmerov | d0ugal: got it, yes | 09:42 |
| rakhmerov | d0ugal: the thing is that part of conversions happen internally, and there are some YAQL options btw that affect that that we (as far as I remember) had to change | 09:45 |
| d0ugal | wow, there really hasn't been a YAQL release for some time | 09:45 |
| d0ugal | https://github.com/openstack/mistral/blob/master/mistral/expressions/yaql_expression.py#L39-L44 | 09:45 |
| d0ugal | ^ some of those engine options are not even released btw | 09:45 |
| d0ugal | so when there is a release, I hope it doesn't change things | 09:45 |
| d0ugal | brb | 09:45 |
| * quiquell|rover takes the backpack and goes back to #tripleo | 09:46 | |
| quiquell|rover | :-) | 09:46 |
| rakhmerov | d0ugal: as far as I can see, all of these options are released | 09:47 |
| rakhmerov | they are all present in our config | 09:47 |
| d0ugal | oh, maybe they just got added to the CLI interface then | 09:50 |
| d0ugal | I was just looking at https://github.com/openstack/yaql/compare/1.1.3...master | 09:51 |
| rakhmerov | ok | 09:59 |
| rakhmerov | one simple fix that I have in mind is to fix standard YAQL functions so that they coerce collections before comparing them | 10:00 |
| rakhmerov | but not sure what else it can break :) | 10:00 |
| d0ugal | That would be a big change in YAQL | 10:06 |
| rakhmerov | yes | 10:08 |
| rakhmerov | I would probably suggest we revert my patch for now, breaking list comparisons seems a worse problem to me than breaking str() | 10:12 |
| rakhmerov | I don't expect to find a good solution within 2-3 days (have to now switch to other things) | 10:13 |
| d0ugal | ok, that sounds like a sensible plan to me | 10:13 |
| rakhmerov | but I'll see what I can do with str() function, maybe there's a way to fix it w/o breaking these comparisons | 10:13 |
| rakhmerov | I spent 2-3 hours yesterday evening debugging YAQL when it evaluates this expression | 10:14 |
| rakhmerov | the logic there is terribly difficult and these internal conversions of list into tuples works in a very implicit way | 10:14 |
| rakhmerov | I found where it happens but it's not configurable in any way from outside | 10:14 |
| rakhmerov | seems like we can only fix YAQL itself | 10:15 |
| rakhmerov | so I guess dealing with "str" would be a better option | 10:15 |
| d0ugal | yup | 10:15 |
| rakhmerov | apetrich, akovi: do you guys agree? | 10:15 |
| rakhmerov | any other ideas? | 10:16 |
| rakhmerov | d0ugal: btw, would you have some time to propose a release of oslo.cache? | 10:16 |
| d0ugal | I guess I can | 10:17 |
| rakhmerov | thanks | 10:18 |
| d0ugal | rakhmerov: it is already released | 10:24 |
| rakhmerov | really? | 10:24 |
| rakhmerov | have a link? | 10:24 |
| rakhmerov | I just checked and didn't find it | 10:24 |
| rakhmerov | maybe overlooked | 10:24 |
| d0ugal | https://github.com/openstack/oslo.cache/tree/1.33.0 | 10:24 |
| d0ugal | 1.33.0 | 10:24 |
| d0ugal | 9 days ago | 10:24 |
| d0ugal | there are no changes in master not in that tag | 10:25 |
| d0ugal | I'm a bit confused | 10:25 |
| d0ugal | it was dogpile we had the issue with, right? | 10:25 |
| rakhmerov | yeah... | 10:27 |
| rakhmerov | wait | 10:27 |
| rakhmerov | but it was uncapped later than 9 days ago, wasn't it? | 10:27 |
| d0ugal | That is why I'm confused | 10:28 |
| d0ugal | https://review.openstack.org/#/c/636037/ | 10:28 |
| d0ugal | Yeah, Feb 18th | 10:28 |
| d0ugal | oh, sorry, the commit was 9 days ago | 10:28 |
| d0ugal | but it was only merged on the 18th | 10:29 |
| d0ugal | The release was made yesterday only | 10:29 |
| d0ugal | https://github.com/openstack/releases/commit/62688fdb54ec434e55333cadf7c30909b5b3de9e | 10:29 |
| d0ugal | https://review.openstack.org/#/c/637609/ | 10:30 |
| d0ugal | Merged just over 12 hours ago | 10:30 |
| rakhmerov | d0ugal: ok | 10:32 |
| rakhmerov | so now our issues with dogpile.cache should go away? :) | 10:33 |
| d0ugal | I hope so! | 10:33 |
| rakhmerov | :) | 10:33 |
| rakhmerov | let's try to recheck then | 10:33 |
| rakhmerov | btw, https://review.openstack.org/#/c/637170 | 10:33 |
| rakhmerov | revert | 10:33 |
| d0ugal | +W, thanks | 10:34 |
| openstackgerrit | Renat Akhmerov proposed openstack/mistral master: Stop using deprecated keystone_authtoken/auth_uri https://review.openstack.org/594187 | 10:37 |
| *** jtomasek has joined #openstack-mistral | 11:32 | |
| rakhmerov | d0ugal: https://review.openstack.org/#/c/637145/ :) | 12:24 |
| rakhmerov | it passed now | 12:24 |
| rakhmerov | let's merge it | 12:24 |
| rakhmerov | apetrich: Adriano, can you please review and approve it? | 12:25 |
| d0ugal | oops, I just did | 12:25 |
| d0ugal | lol | 12:25 |
| d0ugal | phew apetrich agrees ;) | 12:26 |
| apetrich | :) | 12:26 |
| rakhmerov | ok :) | 12:26 |
| rakhmerov | np | 12:26 |
| rakhmerov | thanks | 12:26 |
| openstackgerrit | Merged openstack/mistral master: Revert "Fix how Mistral prepares data for evaluating a YAQL expression" https://review.openstack.org/637170 | 12:35 |
| rakhmerov | aah, https://review.openstack.org/594187 passed too, merging it ) | 12:41 |
| rakhmerov | so our CI is finally unblocked | 12:41 |
| rakhmerov | good | 12:41 |
| apetrich | great | 13:10 |
| d0ugal | unblocked... but for how long? :) | 13:16 |
| *** smrcascao has joined #openstack-mistral | 13:44 | |
| *** smrcascao has quit IRC | 13:56 | |
| *** smrcascao has joined #openstack-mistral | 14:06 | |
| *** jtomasek has quit IRC | 14:17 | |
| *** smrcascao has quit IRC | 14:36 | |
| openstackgerrit | Merged openstack/mistral master: Update dogpile.cache to match global requirements https://review.openstack.org/637145 | 16:25 |
| *** quiquell|rover is now known as quiquell|off | 16:38 | |
| *** smrcascao has joined #openstack-mistral | 16:49 | |
| *** akovi has quit IRC | 16:49 | |
| *** smrcascao has quit IRC | 16:49 | |
| *** pgaxatte has quit IRC | 16:50 | |
| *** smrcascao has joined #openstack-mistral | 16:51 | |
| *** akovi has joined #openstack-mistral | 16:56 | |
| *** akovi has quit IRC | 17:01 | |
| *** smrcascao has quit IRC | 18:10 | |
| *** jtomasek has joined #openstack-mistral | 18:44 | |
| *** openstackgerrit has quit IRC | 20:09 | |
| *** openstackgerrit has joined #openstack-mistral | 21:09 | |
| openstackgerrit | Merged openstack/mistral master: Stop using deprecated keystone_authtoken/auth_uri https://review.openstack.org/594187 | 21:09 |
| *** jtomasek has quit IRC | 21:57 | |
| *** d0ugal has quit IRC | 22:03 | |
| *** d0ugal has joined #openstack-mistral | 22:20 | |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!