<div dir="ltr">I wanted to bump this thread. There’s a PR waiting on a resolution. I see some agreement but not sure what the next steps are?<br clear="all"><div><div dir="ltr" class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><div><br></div><div>David<br></div></div></div></div></div></div></div></div><br></div><br><div class="gmail_quote"><div dir="ltr">On Thu, Aug 9, 2018 at 5:34 PM Jeff Ortel <<a href="mailto:jortel@redhat.com">jortel@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
  
    
  
  <div text="#000000" bgcolor="#FFFFFF">
    <br>
    <br>
    <div class="m_-7926982628397342293moz-cite-prefix">On 08/09/2018 01:29 PM, Daniel Alley
      wrote:<br>
    </div>
    <blockquote type="cite">
      <div dir="ltr">
        <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
          <div dir="ltr">
            <div>It's possible we could want additional sync_modes in
              the future. To me, sync mode deals with the contents of
              the repo during the sync. There are other ways you would
              want to have a sync associate content with a repository.
              Consider a retention behavior that retains 5 versions of
              each unit, e.g. rpms, ansible modules, etc; that behavior
              is somewhere in between mirror and additive. If we make
              mirror a boolean then to introduce this retention feature
              we would have to add it as an additional option. This
              creates the downside I hope to avoid which is that
              interaction between options becomes complicated.</div>
            <div><br>
            </div>
            <div>For example, a call with both (mirror=False,
              retention=True) now becomes more complicated to think
              about. Is it mirroring or using the retention policy? How
              do these interact? At that point, it seems more
              complicated than what we have now. The way to avoid this
              is by keeping them together as one option, but that can
              only be done if it stays as a string.</div>
          </div>
        </blockquote>
        <div><br>
        </div>
        <div>These are all good points but I think "retention" would
          likely need to be a configurable parameter, probably one that
          you would have to pass in.  The default value could mean
          "unlimited retention", i.e.  "additive".<br>
        </div>
        <div><br>
        </div>
        <div>So what you could do is:</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>(mirror=False)                       # this is normal
            additive mode, retain everything.  let's say that default
            retention=0, which is nonsensical and would map to this
            behavior instead<br>
          </div>
        </blockquote>
        <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
          <div>(mirror=False, retention=5)     # retain at most 5
            versions of any given unit<br>
          </div>
        </blockquote>
        <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
          <div>(mirror=False, retention=1)     # this is *almost* like
            mirror mode, except that you would still keep one historical
            copy of units that are no longer present in the upstream
            repository<br>
          </div>
        </blockquote>
        <div><br>
        </div>
        <div>Maybe it even makes sense to have retention be able to
          modify "mirror" mode, although this would make the concept of
          "mirror" more difficult to understand as you point out.  Maybe
          we could find a name that would be less misleading.<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>(mirror=True, retention=5)       # retain at most 5
            versions of any given unit, <i>but purge units that that
              are no longer present in the upstream repo entirely</i></div>
        </blockquote>
      </div>
    </blockquote>
    <br>
    This ^^ matches what I was thinking as well.<br>
    <br>
    <blockquote type="cite">
      <div dir="ltr">
        <div><br>
        </div>
        <div>I don't have a specific use case in mind for that one, but
          maybe someone can think of one?</div>
        <div><br>
        </div>
      </div>
      <div class="gmail_extra"><br>
        <div class="gmail_quote">On Thu, Aug 9, 2018 at 12:53 PM, Brian
          Bouterse <span dir="ltr"><<a href="mailto:bbouters@redhat.com" target="_blank">bbouters@redhat.com</a>></span>
          wrote:<br>
          <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
            <div dir="ltr">
              <div>It's possible we could want additional sync_modes in
                the future. To me, sync mode deals with the contents of
                the repo during the sync. There are other ways you would
                want to have a sync associate content with a repository.
                Consider a retention behavior that retains 5 versions of
                each unit, e.g. rpms, ansible modules, etc; that
                behavior is somewhere in between mirror and additive. If
                we make mirror a boolean then to introduce this
                retention feature we would have to add it as an
                additional option. This creates the downside I hope to
                avoid which is that interaction between options becomes
                complicated.</div>
              <div><br>
              </div>
              <div>For example, a call with both (mirror=False,
                retention=True) now becomes more complicated to think
                about. Is it mirroring or using the retention policy?
                How do these interact? At that point, it seems more
                complicated than what we have now. The way to avoid this
                is by keeping them together as one option, but that can
                only be done if it stays as a string.</div>
            </div>
            <div class="m_-7926982628397342293HOEnZb">
              <div class="m_-7926982628397342293h5">
                <div class="gmail_extra"><br>
                  <div class="gmail_quote">On Thu, Aug 9, 2018 at 9:04
                    AM, Milan Kovacik <span dir="ltr"><<a href="mailto:mkovacik@redhat.com" target="_blank">mkovacik@redhat.com</a>></span>
                    wrote:<br>
                    <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
                      <div dir="ltr"><br>
                        <div class="gmail_extra"><br>
                          <div class="gmail_quote"><span>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>
                            </span>
                            <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>
                            <span>
                              <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>
                            </span>
                            <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%23L114" target="_blank">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" target="_blank">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>
                              _______________________________________________<br>
                              Pulp-dev mailing list<br>
                              <a href="mailto:Pulp-dev@redhat.com" target="_blank">Pulp-dev@redhat.com</a><br>
                              <a href="https://www.redhat.com/mailman/listinfo/pulp-dev" rel="noreferrer" target="_blank">https://www.redhat.com/mailman/listinfo/pulp-dev</a><br>
                              <br>
                            </blockquote>
                          </div>
                          <br>
                        </div>
                      </div>
                      <br>
                      _______________________________________________<br>
                      Pulp-dev mailing list<br>
                      <a href="mailto:Pulp-dev@redhat.com" target="_blank">Pulp-dev@redhat.com</a><br>
                      <a href="https://www.redhat.com/mailman/listinfo/pulp-dev" rel="noreferrer" target="_blank">https://www.redhat.com/mailman/listinfo/pulp-dev</a><br>
                      <br>
                    </blockquote>
                  </div>
                  <br>
                </div>
              </div>
            </div>
            <br>
            _______________________________________________<br>
            Pulp-dev mailing list<br>
            <a href="mailto:Pulp-dev@redhat.com" target="_blank">Pulp-dev@redhat.com</a><br>
            <a href="https://www.redhat.com/mailman/listinfo/pulp-dev" rel="noreferrer" target="_blank">https://www.redhat.com/mailman/listinfo/pulp-dev</a><br>
            <br>
          </blockquote>
        </div>
        <br>
      </div>
      <br>
      <fieldset class="m_-7926982628397342293mimeAttachmentHeader"></fieldset>
      <br>
      <pre>_______________________________________________
Pulp-dev mailing list
<a class="m_-7926982628397342293moz-txt-link-abbreviated" href="mailto:Pulp-dev@redhat.com" target="_blank">Pulp-dev@redhat.com</a>
<a class="m_-7926982628397342293moz-txt-link-freetext" href="https://www.redhat.com/mailman/listinfo/pulp-dev" target="_blank">https://www.redhat.com/mailman/listinfo/pulp-dev</a>
</pre>
    </blockquote>
    <br>
  </div>

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