[Pulp-dev] Revisit: sync modes

Jeff Ortel jortel at redhat.com
Thu Aug 9 21:34:09 UTC 2018



On 08/09/2018 01:29 PM, Daniel Alley wrote:
>
>     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.
>
>     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.
>
>
> 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".
>
> So what you could do is:
>
>     (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
>
>     (mirror=False, retention=5)     # retain at most 5 versions of any
>     given unit
>
>     (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
>
>
> 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.
>
>     (mirror=True, retention=5)       # retain at most 5 versions of
>     any given unit, /but purge units that that are no longer present
>     in the upstream repo entirely/
>

This ^^ matches what I was thinking as well.

>
> I don't have a specific use case in mind for that one, but maybe 
> someone can think of one?
>
>
> On Thu, Aug 9, 2018 at 12:53 PM, Brian Bouterse <bbouters at redhat.com 
> <mailto:bbouters at redhat.com>> wrote:
>
>     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.
>
>     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.
>
>     On Thu, Aug 9, 2018 at 9:04 AM, Milan Kovacik <mkovacik at redhat.com
>     <mailto:mkovacik at redhat.com>> wrote:
>
>
>
>         On Wed, Aug 8, 2018 at 7:54 PM, Jeff Ortel <jortel at redhat.com
>         <mailto:jortel at redhat.com>> wrote:
>
>             I'm not convinced that /named/ 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 /named/ 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
>             /modes/ and /policies/ but given the orthogonal nature of
>             the two modes we currently support _and_ that no /real/ or
>             anticipated use cases for additional modes are known, I'm
>             not convinced it's a good fit.  Are there any /real/ or
>             anticipated use cases I'm missing?
>
>
>         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.
>         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.
>
>
>             I propose we replace the (str)sync_mode="" with
>             (bool)mirror=False anywhere stored or passed.
>
>             Are there any /real/ or anticipated use cases I'm missing?
>
>             Thoughts?
>
>
>         I'm afraid replacing custom strings with True/False won't make
>         the situation much better.
>         I'd vote for some refactor besides other things, it might
>         better be part of remote (or repository) creation endpoint.
>
>         Cheers,
>         milan
>         [1]
>         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%23L114>
>         [2]
>         https://github.com/pulp/pulp/pull/3583#discussion_r208869824
>         <https://github.com/pulp/pulp/pull/3583#discussion_r208869824>
>
>
>
>
>             _______________________________________________
>             Pulp-dev mailing list
>             Pulp-dev at redhat.com <mailto:Pulp-dev at redhat.com>
>             https://www.redhat.com/mailman/listinfo/pulp-dev
>             <https://www.redhat.com/mailman/listinfo/pulp-dev>
>
>
>
>         _______________________________________________
>         Pulp-dev mailing list
>         Pulp-dev at redhat.com <mailto:Pulp-dev at redhat.com>
>         https://www.redhat.com/mailman/listinfo/pulp-dev
>         <https://www.redhat.com/mailman/listinfo/pulp-dev>
>
>
>
>     _______________________________________________
>     Pulp-dev mailing list
>     Pulp-dev at redhat.com <mailto:Pulp-dev at redhat.com>
>     https://www.redhat.com/mailman/listinfo/pulp-dev
>     <https://www.redhat.com/mailman/listinfo/pulp-dev>
>
>
>
>
> _______________________________________________
> Pulp-dev mailing list
> Pulp-dev at redhat.com
> https://www.redhat.com/mailman/listinfo/pulp-dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/pulp-dev/attachments/20180809/2b03e181/attachment.htm>


More information about the Pulp-dev mailing list