<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Aug 8, 2018 at 7:54 PM, Jeff Ortel <span dir="ltr"><<a href="mailto:jortel@redhat.com" target="_blank">jortel@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
  

    
  
  <div bgcolor="#FFFFFF">
    I'm not convinced that <i>named</i> sync mode is a good approach. 
    I doubt it will ever be anything besides (additive|mirror) which
    really boils down to mirror (or not).  Perhaps the reasoning behind
    a <i>named</i> mode is that it is potentially more extensible in
    that the API won't be impacted when a new mode is needed.  The main
    problem with this approach is that the mode names are validated and
    interpreted in multiple places. Adding another mode will require
    coordinated changes in both the core and most plugins.  Generally,
    I'm an advocate of named things like <i>modes</i> and <i>policies</i>
    but given the orthogonal nature of the two modes we currently
    support <u>and</u> that no <i>real</i> or anticipated use cases
    for additional modes are known, I'm not convinced it's a good fit. 
    Are there any <i>real</i> or anticipated use cases I'm missing?<br></div></blockquote><div><br></div><div>Looking at the code[1] we're actually talking about almost a (pipeline) factory that has exactly 2 modes of operation with a limited possibilities of extending, unsure that the possibility to extend was a goal though. <br></div><div>Moreover it turns out current implementation prevents using (class-level) constants instead of custom strings due to plugin--platform import issues: core serializer can't refer to DeclarativeVersion.defaul_sync_mode --- at least I wasn't able to make this work as part of the sync_mode docstring PR[2] review suggestion.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div bgcolor="#FFFFFF">
    <br>
    I propose we replace the (str)sync_mode="" with (bool)mirror=False
    anywhere stored or passed.<br>
    <br>
    Are there any <i>real</i> or anticipated use cases I'm missing?<br>
    <br>
    Thoughts?<br></div></blockquote><div><br></div><div>I'm afraid replacing custom strings with True/False won't make the situation much better.</div><div>I'd vote for some refactor besides other things, it might better be part of remote (or repository) creation endpoint.<br></div><div><br></div><div>Cheers,</div><div>milan<br></div><div> </div><div><div>[1] <a href="https://github.com/pulp/pulp/blob/master/plugin/pulpcore/plugin/stages/declarative_version.py#L100#L114">https://github.com/pulp/pulp/blob/master/plugin/pulpcore/plugin/stages/declarative_version.py#L100#L114</a></div><div>[2] <a href="https://github.com/pulp/pulp/pull/3583#discussion_r208869824">https://github.com/pulp/pulp/pull/3583#discussion_r208869824</a><br></div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div bgcolor="#FFFFFF">
    <br>
  </div>

<br>______________________________<wbr>_________________<br>
Pulp-dev mailing list<br>
<a href="mailto:Pulp-dev@redhat.com">Pulp-dev@redhat.com</a><br>
<a href="https://www.redhat.com/mailman/listinfo/pulp-dev" rel="noreferrer" target="_blank">https://www.redhat.com/<wbr>mailman/listinfo/pulp-dev</a><br>
<br></blockquote></div><br></div></div>