[Pulp-dev] Revisit: sync modes

Thu Aug 9 20:24:36 UTC 2018

On Thu, Aug 9, 2018 at 8:29 PM, Daniel Alley <dalley 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.
>>
>
> 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*
>>
>
> I don't have a specific use case in mind for that one, but maybe someone
> can think of one?
>

Anyone has a suggestion beyond 'mirror' and 'retention' vs. 'additive'
modes?
If we anticipate bigger sync mode variability  we should probably refactor
the DeclarativeVersion into a proper pipeline factory.
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.

>
> On Thu, Aug 9, 2018 at 12:53 PM, Brian Bouterse <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>
>> wrote:
>>
>>>
>>>
>>> On Wed, Aug 8, 2018 at 7:54 PM, Jeff Ortel <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/plu
>>> gin/stages/declarative_version.py#L100#L114
>>> [2] https://github.com/pulp/pulp/pull/3583#discussion_r208869824
>>>
>>>
>>>
>>>>
>>>> _______________________________________________
>>>> Pulp-dev mailing list
>>>> Pulp-dev at redhat.com
>>>> 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
>>>
>>>
>>
>> _______________________________________________
>> 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/de76fdac/attachment.htm>