| *** jamielennox is now known as jamielennox|away | 00:58 | |
| *** jamielennox|away is now known as jamielennox | 01:12 | |
| *** zhurong has joined #openstack-mistral | 01:32 | |
| *** bobh has quit IRC | 02:12 | |
| *** bobh has joined #openstack-mistral | 02:20 | |
| *** bobh has quit IRC | 02:36 | |
| *** gongysh has joined #openstack-mistral | 02:40 | |
| *** bobh has joined #openstack-mistral | 02:41 | |
| *** dnalezyty has quit IRC | 02:59 | |
| *** bobh has quit IRC | 03:16 | |
| *** gongysh has quit IRC | 04:40 | |
| *** gongysh has joined #openstack-mistral | 04:54 | |
| *** gongysh has quit IRC | 04:56 | |
| *** gongysh has joined #openstack-mistral | 05:51 | |
| *** shardy has joined #openstack-mistral | 08:04 | |
| rakhmerov | d0ugal, mgershen, ddeja, kong: please don't forget to review stuff :) | 08:23 |
|---|---|---|
| d0ugal | I do try! | 08:23 |
| *** jaosorior has joined #openstack-mistral | 08:24 | |
| rakhmerov | thanks | 08:37 |
| rakhmerov | d0ugal, mgershen: I'm specifically interested very much in: https://review.openstack.org/444732, | 08:37 |
| rakhmerov | https://review.openstack.org/444791 and https://review.openstack.org/444845 | 08:38 |
| *** amoralej|off is now known as amoralej | 08:39 | |
| *** gongysh has quit IRC | 08:45 | |
| openstackgerrit | Dougal Matthews proposed openstack/mistral master: Don't create actions with empty arg_lists https://review.openstack.org/412433 | 08:58 |
| *** jpich has joined #openstack-mistral | 08:58 | |
| *** gongysh has joined #openstack-mistral | 09:12 | |
| *** gongysh has quit IRC | 09:33 | |
| *** therve has joined #openstack-mistral | 09:38 | |
| therve | d0ugal, Hi! Thanks for understanding my response :) | 09:39 |
| therve | I didn't think I was particularly clear | 09:39 |
| therve | d0ugal, I thought the context was extracted from some thread local variables, is it not the caseN | 09:40 |
| therve | ? | 09:40 |
| therve | https://github.com/openstack/mistral/blob/master/mistral/actions/openstack/actions.py#L76 uses context.ctx(), not some arguments | 09:40 |
| d0ugal | therve: it is the case now, but with the move to mistral-lib it will change | 09:46 |
| d0ugal | therve: https://review.openstack.org/#/c/411412/ | 09:47 |
| d0ugal | therve: tl;dr - to make it easier to write custom actions you will depend on mistral-lib, not mistral. Rather than moving the context to mistral-lib it will be passedd to the action by Mistral | 09:48 |
| d0ugal | therve: and the openstack actions are going to move to mistral-extra (and use mistral-lib - so they can't use the private context access they use now) | 09:50 |
| therve | OK that makes sense | 09:51 |
| therve | d0ugal, Then you can pass the context as an argument? | 09:52 |
| d0ugal | therve: Yeah, the current proposal is to pass it to the run method. See https://review.openstack.org/#/c/411412/11/mistral_lib/actions/base.py | 09:53 |
| d0ugal | Line 42 | 09:53 |
| therve | d0ugal, Right, I was thinking about the get_client methods | 09:53 |
| d0ugal | therve: gotcha, that isn't fully planned yet :) | 09:53 |
| *** zhurong has quit IRC | 09:54 | |
| d0ugal | therve: I have started writing a spec for this work. | 09:55 |
| therve | Cool | 09:55 |
| therve | d0ugal, I'm happy to help, if I can spread the message that multiple inheritance is evil :) | 09:55 |
| d0ugal | spread? so you are pro-multiple inheritance? :) | 09:56 |
| therve | Quite the opposite :) | 09:57 |
| d0ugal | oh, sorry, I miss-read | 09:57 |
| d0ugal | I thought you said you want to spread multiple inheritance! | 09:57 |
| d0ugal | (not that you want to spread the message) | 09:57 |
| therve | :D | 09:57 |
| d0ugal | therve: FWIW I generally agree with you, I do think there is value in mixins in some situations but I think the cost is higher than the value add for us | 09:58 |
| d0ugal | more complexity and harder to document | 09:58 |
| therve | functions >>> mixins | 09:59 |
| d0ugal | heh :) | 09:59 |
| therve | More testable, clear interface | 09:59 |
| d0ugal | +1 | 09:59 |
| d0ugal | therve: your example could just become something like "NovaAction().get_client(context)" | 10:01 |
| therve | d0ugal, Right | 10:01 |
| d0ugal | or NovaAction.client(context) <- make it a class method and remove the superfluous "get_" | 10:02 |
| therve | Yeah there are various ways to make it work | 10:02 |
| therve | The point it, if you use mixin you probably don't want to have state variables on all the classes | 10:03 |
| therve | And if you don't, you're better of with pure functions (or static/class method if you need some nesting) | 10:03 |
| d0ugal | makes sense | 10:04 |
| d0ugal | rakhmerov: ^ that may be of interest to you | 10:08 |
| d0ugal | rakhmerov: The meeting time change merged - other than sending an email to openstack-dev, do you know what else I need to do? | 10:08 |
| d0ugal | oh, the wiki maybe... | 10:08 |
| rakhmerov | d0ugal: at a meeting now.. will respond later | 10:13 |
| d0ugal | rakhmerov: np, thanks | 10:14 |
| *** jkilpatr has quit IRC | 10:39 | |
| rakhmerov | d0ugal: yep, wiki and ML, I think that's all that's needed | 10:46 |
| d0ugal | rakhmerov: cool, I updated the wiki and I'll send out the email now | 10:46 |
| rakhmerov | therve, d0ugal: yeah, agree mostly on mixins (and state) | 10:46 |
| d0ugal | rakhmerov: or do you want to do the email and include the calendar invite? | 10:46 |
| rakhmerov | d0ugal: ooh, yeah, let me do this | 10:47 |
| rakhmerov | therve: I'm not sure though I clearly understand your idea about functions | 10:47 |
| rakhmerov | I saw you wrote more in ML, let me read it first.. | 10:48 |
| d0ugal | rakhmerov: my last email may have made it clearer (or maybe not!) :) | 10:50 |
| rakhmerov | reading.. | 10:50 |
| d0ugal | rakhmerov: this is what I was going to send: http://paste.openstack.org/show/602659/ - but I'll just let you send the calendar invite instead | 10:53 |
| rakhmerov | thanks, ok | 10:54 |
| openstackgerrit | Merged openstack/mistral master: Updated from global requirements https://review.openstack.org/445093 | 10:54 |
| rakhmerov | so, just read last 4 messages in ML | 10:54 |
| rakhmerov | now I see that we're discussing too different things IMO | 10:55 |
| rakhmerov | I was mostly talking about base actions framework rather than a way to refactor particularly OpenStack actions | 10:56 |
| rakhmerov | as far as OpenStack actions: I understand the problem, agree with the NovaAction.get_client(context) solution. So here I think we shouldn't use multiple inheritance just to have access to two different clients | 10:57 |
| rakhmerov | seems pointless to me | 10:58 |
| d0ugal | rakhmerov: should mistral-extra be considered a Python library? | 10:59 |
| d0ugal | rakhmerov: do we want people to depend on it and import the actions? | 10:59 |
| d0ugal | or do we just want it to be an optional component | 11:00 |
| rakhmerov | hm.. | 11:00 |
| rakhmerov | why not make it possible to depend on it? | 11:00 |
| rakhmerov | just thinking.. | 11:01 |
| rakhmerov | I think yes, it should be a library also | 11:01 |
| d0ugal | Me too :) | 11:01 |
| rakhmerov | I may want to take some of the action implementations, for example, and extend them, alter their behavior | 11:01 |
| d0ugal | rakhmerov: yup, agreed | 11:04 |
| d0ugal | I think it should be a library, but I wasn't sure if I was too biased because of TripleO | 11:04 |
| d0ugal | I will continue down that direction in the spec | 11:04 |
| rakhmerov | yep | 11:14 |
| rakhmerov | btw, just replied in the email | 11:15 |
| rakhmerov | hopefully we now understand each other ) | 11:15 |
| *** jkilpatr has joined #openstack-mistral | 11:19 | |
| *** jkilpatr has quit IRC | 11:27 | |
| d0ugal | rakhmerov: thanks for the reply, I think that makes sense to me. I agree fully | 11:38 |
| rakhmerov | ok | 11:38 |
| *** jkilpatr has joined #openstack-mistral | 11:40 | |
| openstackgerrit | Dougal Matthews proposed openstack/mistral-lib master: Fix the package name in the docs https://review.openstack.org/445455 | 12:00 |
| openstackgerrit | Dougal Matthews proposed openstack/mistral-lib master: Fix the package name in the coveragerc https://review.openstack.org/445456 | 12:00 |
| openstackgerrit | Dougal Matthews proposed openstack/mistral-lib master: Fix the package name in the setup.cfg https://review.openstack.org/445457 | 12:00 |
| d0ugal | rakhmerov: ^ I spotted a few issues with naming in mistral-lib. | 12:00 |
| rakhmerov | ok | 12:00 |
| rakhmerov | I'll look at it | 12:00 |
| d0ugal | mgershen: btw, I think it is okay to +2 before the tests pass :) but it is up to you | 12:01 |
| rakhmerov | done | 12:02 |
| rakhmerov | yeah, I usually approve even before the tests pass. If they fail the patch will block anyway.. | 12:02 |
| d0ugal | Interesting, I don't do that - but mostly because TripleO CI is a bit special :) | 12:03 |
| d0ugal | so I probably could in Mistral | 12:03 |
| *** gongysh has joined #openstack-mistral | 12:37 | |
| *** dprince has joined #openstack-mistral | 12:49 | |
| *** amoralej is now known as amoralej|lunch | 12:53 | |
| *** catintheroof has joined #openstack-mistral | 12:58 | |
| *** catintheroof has quit IRC | 12:59 | |
| *** catintheroof has joined #openstack-mistral | 12:59 | |
| *** gongysh has quit IRC | 13:03 | |
| *** chlong has joined #openstack-mistral | 13:04 | |
| openstackgerrit | Merged openstack/mistral-lib master: Fix the package name in the docs https://review.openstack.org/445455 | 13:05 |
| openstackgerrit | Istvan Imre proposed openstack/mistral master: Log stack trace if action initialization faild https://review.openstack.org/445484 | 13:05 |
| *** fultonj has joined #openstack-mistral | 13:11 | |
| *** catinthe_ has joined #openstack-mistral | 13:41 | |
| *** catintheroof has quit IRC | 13:41 | |
| *** catintheroof has joined #openstack-mistral | 13:45 | |
| *** catinthe_ has quit IRC | 13:45 | |
| *** IRCFrEAK has joined #openstack-mistral | 13:46 | |
| *** IRCFrEAK has quit IRC | 13:48 | |
| *** amoralej|lunch is now known as amoralej | 14:08 | |
| *** toure|gone is now known as toure | 14:13 | |
| openstackgerrit | Ryan Brady proposed openstack/mistral-lib master: Adds minimum common shared code for custom actions API https://review.openstack.org/411412 | 14:18 |
| toure | d0ugal rakhmerov good morning/evening are my proposed changes +1 or -1 or off the deep end :) | 14:34 |
| *** bobh has joined #openstack-mistral | 14:34 | |
| d0ugal | toure: oh, I've not looked yet | 14:34 |
| d0ugal | toure: which changes? | 14:35 |
| toure | https://review.openstack.org/#/c/443217/ | 14:35 |
| toure | error analysis | 14:35 |
| toure | thanks | 14:35 |
| d0ugal | toure: oh, the spec. I'll take a look | 14:35 |
| toure | yup | 14:35 |
| toure | thanks | 14:35 |
| d0ugal | I actually don't know that much about this one, so I might have some silly questions. | 14:35 |
| toure | no silly questions, just silly answers | 14:36 |
| toure | :) | 14:36 |
| d0ugal | lol | 14:36 |
| *** shardy has quit IRC | 14:37 | |
| *** shardy has joined #openstack-mistral | 14:37 | |
| openstackgerrit | Merged openstack/mistral-lib master: Fix the package name in the coveragerc https://review.openstack.org/445456 | 14:38 |
| d0ugal | toure: What would the input to this command be? The execution ID? | 14:40 |
| toure | I am going back an forth on that | 14:41 |
| toure | should it be something which the method searches for in regards to the state | 14:41 |
| toure | or should the user provide the id | 14:41 |
| toure | d0ugal I will probably go with the simpler solution and ask the user to provide the id | 14:42 |
| d0ugal | toure: yeah, I assume the user would pass the ID | 14:43 |
| toure | yeah | 14:43 |
| toure | also I changed the verbiage of the command | 14:43 |
| d0ugal | toure: we have other commands like "mistral execution-get $ID" and "mistral execution-get-output $ID" | 14:43 |
| d0ugal | so I wondered if it makes sense to group it with them | 14:43 |
| d0ugal | "mistral exeuction-get-errors $ID" or something | 14:44 |
| d0ugal | I'm just thinking "out loud" at the moment :) | 14:44 |
| toure | well I was thinking if we wanted to expand the functionality of the reporting method the new verbiage would allow that flexibility | 14:45 |
| toure | "mistral report generate" | 14:45 |
| toure | "mistral report list" | 14:45 |
| toure | if we want to store historical data | 14:45 |
| d0ugal | toure: right, makes sense | 14:46 |
| toure | d0ugal do you like the draft output? | 14:47 |
| toure | maybe missing something? | 14:47 |
| d0ugal | toure: Still processing it a bit :) | 14:47 |
| toure | but I figured I should keep it concise | 14:47 |
| toure | :) | 14:47 |
| toure | kk | 14:47 |
| d0ugal | toure: I think the workflow structure could be problematic, that could be large | 14:48 |
| d0ugal | toure: I'm also not sure how the marker would work | 14:48 |
| d0ugal | toure: I don't think there is a marker in the example? | 14:48 |
| toure | I am looking into that part | 14:48 |
| d0ugal | toure: Once Mistral has parsed the YAML and created the internal representation of the workflow it is tricky to go back to the original definition and point to a location | 14:49 |
| toure | true | 14:49 |
| * toure drops marker and workflow dump | 14:49 | |
| d0ugal | toure: You don't need to drop the idea, it should be possible - I just think it could also be tricky :) | 14:50 |
| toure | kk | 14:50 |
| d0ugal | toure: I was trying to do something recently when writing a linter for Mistral :) | 14:51 |
| toure | cool, that is what I was thinking of writing | 14:52 |
| toure | :) | 14:52 |
| d0ugal | toure: https://github.com/d0ugal/mistral-lint | 14:52 |
| d0ugal | toure: I think it would be useful to include the task input/output and the action input/output | 14:52 |
| toure | cool | 14:52 |
| toure | yeah | 14:52 |
| toure | so we get the offending variable from the error output | 14:53 |
| * toure inside voice | 14:53 | |
| d0ugal | toure: but I think it generally looks good - I suspect that we will need to play with the output a bit and tweak it once we see real data | 14:54 |
| toure | yeah | 14:54 |
| d0ugal | toure: so maybe we could add some flags to filter/expand the output. | 14:54 |
| d0ugal | toure: mistral report-generate --include-workflow | 14:54 |
| d0ugal | maybe that would include the structure and the marker if we can do it | 14:54 |
| d0ugal | ... for example | 14:54 |
| toure | that is a good idea | 14:54 |
| toure | maybe a different object name instead of report? | 14:55 |
| d0ugal | toure: you know what could be cool? Something a bit like a stack trace. | 14:55 |
| toure | ack | 14:56 |
| d0ugal | I'm not sure what it would look like, but it might be useful | 14:56 |
| * d0ugal attempts to mock an example | 14:56 | |
| toure | d0ugal so instead of traceback output could we report a enumerated diag list, kinda like your linter | 14:57 |
| toure | E101: bad input for var: foo with {"blah nah ha"} | 14:58 |
| toure | E101: bad input for variable foo with {"blah nah ha"} | 14:59 |
| toure | brb..going to get some water | 14:59 |
| d0ugal | toure: http://paste.openstack.org/show/602697/ | 15:00 |
| d0ugal | toure: so if we just displayed something like a tree with the minimum information then you could see why it failed | 15:01 |
| d0ugal | toure: so in that example the main workflow failed because task_b in the sub-workflow failed | 15:01 |
| d0ugal | for large executions we could have an option to only show errored tasks/workflows | 15:01 |
| toure | I like it | 15:03 |
| toure | so that is what I was kinda getting at in the draft | 15:04 |
| d0ugal | toure: and that should be quite easy to generate I think | 15:04 |
| d0ugal | famous last words :-D | 15:04 |
| toure | :) | 15:04 |
| toure | 99% of the code will be written and that will be the 1% which take forever to get right | 15:05 |
| toure | :) | 15:05 |
| d0ugal | haha, indeed | 15:05 |
| d0ugal | we can all have fun coming up with obscure workflows for it :) | 15:05 |
| d0ugal | toure: do you want me to copy this ^ onto the review? or is it fine here? | 15:05 |
| toure | yes please | 15:06 |
| toure | thanks for the idea | 15:06 |
| *** fultonj has quit IRC | 15:49 | |
| *** fultonj has joined #openstack-mistral | 15:57 | |
| *** jkilpatr has quit IRC | 16:10 | |
| *** jkilpatr has joined #openstack-mistral | 16:23 | |
| *** rbrady is now known as rbrady-afk | 16:48 | |
| *** bobh has quit IRC | 16:53 | |
| *** d0ugal has quit IRC | 16:55 | |
| *** jaosorior has quit IRC | 17:45 | |
| *** jpich has quit IRC | 17:45 | |
| *** bobh has joined #openstack-mistral | 17:54 | |
| *** bobh has quit IRC | 17:58 | |
| *** d0ugal has joined #openstack-mistral | 17:59 | |
| *** d0ugal has quit IRC | 18:04 | |
| *** bobh has joined #openstack-mistral | 18:05 | |
| *** rbrady-afk is now known as rbrady | 18:09 | |
| *** jkilpatr has quit IRC | 18:12 | |
| *** jkilpatr has joined #openstack-mistral | 18:13 | |
| *** dprince has quit IRC | 18:38 | |
| *** rbrady is now known as rbrady-afk | 18:38 | |
| *** shardy is now known as shardy_afk | 18:45 | |
| *** amoralej is now known as amoralej|off | 19:43 | |
| *** dprince has joined #openstack-mistral | 20:35 | |
| *** jkilpatr has quit IRC | 20:37 | |
| openstackgerrit | Toure Dunnon proposed openstack/mistral-specs master: [WIP] Workflow Error Analysis https://review.openstack.org/443217 | 20:38 |
| *** jkilpatr has joined #openstack-mistral | 20:58 | |
| *** toure is now known as toure|gone | 21:01 | |
| *** fultonj has quit IRC | 21:17 | |
| *** catintheroof has quit IRC | 21:46 | |
| *** dprince has quit IRC | 21:56 | |
| *** bobh has quit IRC | 22:12 | |
| *** thrash is now known as thrash|g0ne | 22:13 | |
| *** bobh has joined #openstack-mistral | 23:13 | |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!