<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Aug 9, 2018 at 8:29 PM, Daniel Alley <span dir="ltr"><<a href="mailto:dalley@redhat.com" target="_blank">dalley@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"><span class=""><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></blockquote></span></div></blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><span class=""><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><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></span><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) <wbr> # 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><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></blockquote><div> </div><div>Anyone has a suggestion beyond 'mirror' and 'retention' vs. 'additive' modes? <br></div><div>If we anticipate bigger sync mode variability we should probably refactor the DeclarativeVersion into a proper pipeline factory.<br></div><div>Btw I don't think it make sense to change the sync behaviour of a repo with every sync call; I guess we'd better make the sync pipeline construction a remote create time option.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div></div></div><div class="HOEnZb"><div class="h5"><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_3931537132231443749HOEnZb"><div class="m_3931537132231443749h5"><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<wbr>_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/b<wbr>lob/master/plugin/pulpcore/plu<wbr>gin/stages/declarative_version<wbr>.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/p<wbr>ull/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" 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<wbr>/listinfo/pulp-dev</a><br>
<br></blockquote></div><br></div></div>
<br>______________________________<wbr>_________________<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<wbr>/listinfo/pulp-dev</a><br>
<br></blockquote></div><br></div>
</div></div><br>______________________________<wbr>_________________<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<wbr>/listinfo/pulp-dev</a><br>
<br></blockquote></div><br></div>
</div></div></blockquote></div><br></div></div>