| openstackgerrit | Wayne Warren proposed openstack-infra/jenkins-job-builder master: Fix macro expansion in conditional step builder. https://review.openstack.org/503771 | 05:36 |
|---|---|---|
| waynr | whoops, forgot to update that | 05:37 |
| waynr | that solution is ugly as heck | 05:37 |
| waynr | i'm also pretty sure there is at least one other place we need to worry about expanding build steps in a conditional thing | 05:38 |
| waynr | conditional publisher | 05:38 |
| waynr | or something | 05:38 |
| *** hashar has joined #openstack-jjb | 08:13 | |
| *** electrofelix has joined #openstack-jjb | 08:47 | |
| Odd_Bloke | waynr: I don't really know what the motivation for the change was; is there a reason registry.dispatch has stopped doing macro expansion? | 13:29 |
| Odd_Bloke | (I'm wondering if a reference to either the parser or the macro registry could be passed to the module registry, to allow it to continue doing it?) | 13:29 |
| Odd_Bloke | waynr: An extremely rough thing that works: http://paste.ubuntu.com/25540520/ | 13:30 |
| Odd_Bloke | s/works/works for the trivial example I gave before/ | 13:30 |
| waynr | the motivation behind JJB 2.0 is to disentangle the object model of JJB in order to make it useful as something other than a command line tool, ie a library in a different command line tool's implementation | 13:55 |
| Odd_Bloke | Right. | 13:56 |
| Odd_Bloke | So a reference to the parser is Right Out. :p | 13:56 |
| waynr | the major JJB-as-a-library use case I had in mind when I began this work was to make the use of JJB YAML optional, or to be able to rewrite the yamlparser entirely while maintaining direct backwards compatibility with the use of config options | 13:57 |
| waynr | yeah ;) | 13:57 |
| Odd_Bloke | Giving the ModuleRegistry an optional MacroRegistry might still be OK? | 13:58 |
| waynr | well the ideal in my opinion is for the xml builder to be passed the fully macro-expanded and reified data structure at the time of xml building because it reduces the xml builder's dependency on details of how yaml parsing and macro expansion happen | 14:02 |
| waynr | but then again the result as seen in my fix could be argued to be a kind of action-at-a-distance where plugin-specific behavior has leaked out of the module | 14:05 |
| waynr | i suppose it's probably okay to give the module registry a reference to the macro registry since it would only ever be used in the case that there are macros left to expand at xml building time and if someone wanted to rewrite the yaml parser implementation all that would be necessary is to expand macros before getting to the xml building stage of the application | 14:07 |
| *** hashar is now known as hasharAway | 14:44 | |
| Odd_Bloke | Yeah, the logic being so far away from the conditional step builder in your solution isn't great. | 14:44 |
| Odd_Bloke | (And doesn't lend itself to extensibility.) | 14:44 |
| Odd_Bloke | If builders(/other things) could somehow say "anything at this YAML path in my configuration should be expanded as a (builder|...)", then it could remain in distinct passes. | 14:47 |
| Odd_Bloke | And then a parser reimplementation would just have to ensure it handled that correctly. | 14:47 |
| waynr | well again, the ideal is that the xml builder doesn't care where the data from which it is building xml originates | 14:58 |
| waynr | but i don't think including a macro registry reference in the module registry violates that principle | 15:00 |
| Odd_Bloke | It seems like having two phases that each builder handles would help; the first phase transforms "raw" YAML to reified YAML, and the second transforms reified YAML to XML. | 15:44 |
| Odd_Bloke | The problem is that in the XML generation phase, conditional_step has to reach back in to the parsing phase at the moment. | 15:45 |
| waynr | well it doesn't have to actually | 15:47 |
| waynr | reaching back into the parser phase is only one way of accomplishing that goal | 15:48 |
| waynr | giving the macro registry the ability to expand macros in places other than the standard top-level job components is another way (see my current fix) | 15:49 |
| waynr | JJB 2 (and not JJB 1.x) already conceptually divides transformation of raw yaml to reified yaml (we call it "parsing") from the transoformation of a recognizable data structure into xml | 15:55 |
| waynr | reaching back into the yaml parser from the xml generation modules breaks that divide | 15:55 |
| waynr | well, "bridges" might be a better word | 15:55 |
| waynr | so i guess from the point of view of conceptually dividing yaml parsing/expansion from xml generation, reaching back into the yaml parser doesn't accomplish that goal...only implementation of macro expansion at different nesting points within the job data structure does | 15:57 |
| waynr | (taking back my comment at 15:48:13 UTC) | 15:58 |
| Odd_Bloke | Right, your implementation does avoid the reaching-back. | 16:23 |
| Odd_Bloke | (Though I think jj.modules.builders.create_builders is still reaching back, really.) | 16:24 |
| *** electrofelix has quit IRC | 17:05 | |
| *** hasharAway has quit IRC | 17:35 | |
| *** hashar has joined #openstack-jjb | 17:35 | |
| *** hashar has quit IRC | 22:10 | |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!