<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On 1 March 2016 at 10:08, David Caro <span dir="ltr"><<a href="mailto:dcaro@redhat.com" target="_blank">dcaro@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class="">On 02/17 16:52, Thanh Ha wrote:<br>
> On 16 February 2016 at 10:03, Darragh Bailey <<a href="mailto:daragh.bailey@gmail.com">daragh.bailey@gmail.com</a>><br>
> wrote:<br>
><br>
> > Think it all comes down to the following:<br>
> > * Need to understand what exactly is happening within Jenkins with<br>
> > regard to XML updating, clearly not just taking the XML given and<br>
> > changing to match that, more likely some kind of merge is going on.<br>
> ><br>
><br>
> Agreed. I'll try to reach out to the Jenkins team and see if I can get a<br>
> statement from them about how this is supposed to work.<br>
><br>
><br>
> > * Is the sort of workflow described by David limited to the disabled<br>
> > tag, or useful elsewhere? If limited I think we should handle<br>
> > 'disabled' as a special case.<br>
> ><br>
><br>
> Agreed. I think we should handle it specially and making sure it's<br>
> documented. I do see it being useful for "disabled". David can you confirm<br>
> if there's other fields you believe needs to be handled specially like this<br>
> one?<br>
<br>
<br>
</span>The description of the job too, both of them are the ones we use on our flow,<br>
as you can't actually add a comment to the disabled status.</blockquote><div><br></div><div>Good to know. We'll make a note to treat both those fields specially. </div><div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><div class="h5">
> > * If the decision is to change behaviour going forward, initially<br>
> > needs to only apply to new settings being added and have a way of<br>
> > converting existing code while matching existing behaviour via some<br>
> > setting.<br>
> ><br>
> > Expect we'll have to support existing use of optionals for at least<br>
> > one major release for the current modules whether we decide to switch<br>
> > the behaviour for the future or not. So the change identified will<br>
> > have to keep that in consideration.<br>
> ><br>
><br>
> I'm not sure how much the impact is but unless you ever purposely set an<br>
> optional setting to something previously, the optional parameter in Jenkins<br>
> is the default setting already just not explicitly. I've only ever been hit<br>
> with this issue when I set something on purpose and decide later that I no<br>
> longer want it and delete the setting, expecting it to revert to the<br>
> default.<br>
><br>
> I'm not sure how best to handle this as it is a behaviour change and would<br>
> be difficult to support 2 behaviours at the same time as it's hard to tell<br>
> which one should be used. One thought is if we're going with the<br>
> discussions from patch <a href="https://review.openstack.org/261620/" rel="noreferrer" target="_blank">https://review.openstack.org/261620/</a><br>
><br>
> 1. Put a single warning when JJB completes running that states something<br>
> like:<br>
><br>
>     ATTENTION: The behaviour of parameters marked as (optional) is<br>
> changing. In future releases if not passed they will revert to a default<br>
> setting.<br>
><br>
> 2. We can add the "fail_required=False" parameter to the convert_mapping_to_xml<br>
> function as suggested<br>
><br>
> This leaves existing functionality for plugins that use the<br>
> convert_mapping_to_xml function at least to stay the same and for all new<br>
> plugins set "fail_required=True", after a few releases we can flip the<br>
> switch so that it's True by default.<br>
><br>
> 3. We start converting plugins using convert_mapping_to_xml function with<br>
> the fail_required=True setting and set correct "defaults" for optional<br>
> settings<br>
<br>
</div></div>This would mean that the jenkins plugins defaults would be overriden by<br>
whatever jjb thinks is the default no? Would require to keep track of those<br>
default values on the jjb side to match the versions of the plugins?<br></blockquote><div><br></div><div>Yes, however most of the plugins do this today already anyway so there's no change in behaviour except for the percentage of those that allow optional fields which I've found to be a much smaller percentage. In fact in some cases the docs incorrectly state that a field is optional yet the code does a data.get(value, default) indicating that it's actually setting a default.</div><div><br></div><div>We'd depend on the plugin writers to set the defaults to be the same as the Jenkins plugins. This is what's already happening today so I see no difference here. We now have a convert_mapping_to_xml() function which significantly simplifies code related to jenkins plugins so should make it much easier and less intimidating for someone to fix the defaults if they find it incorrect. You can see an example here:</div><div><br></div><div><a href="https://github.com/openstack-infra/jenkins-job-builder/blob/master/jenkins_jobs/modules/builders.py#L2485">https://github.com/openstack-infra/jenkins-job-builder/blob/master/jenkins_jobs/modules/builders.py#L2485</a><br></div><div><br></div><div>As I mentioned before I have an intern coming on this summer who I plan to task with converting our code base to using this function when defining plugin support.</div><div><br></div><div>Regards,</div><div>Thanh</div></div><br></div></div>