[Pulp-dev] Revisit: sync modes

Tue Aug 28 21:39:52 UTC 2018

That makes sense to me.

@vdusek, would you be interested in filing an issue and working on this
since you were previously working on the sync_mode stuff?

David

On Tue, Aug 28, 2018 at 5:06 PM Brian Bouterse <bbouters at redhat.com> wrote:

> I couldn't come up with any more use cases that would motivate keeping
> sync_mode over mirror. As pointed out, even retention would likely be its
> own option. I'm now +1 on making the API and DeclarativeVersion use
> mirror=False.
>
> I think I was the main dissenting opinion on this change, so I believe
> this represents consensus. If there are any -1's please raise them on-list.
> Otherwise I would say that PR and any follow-up work can go ahead and
> happen.
>
> On Tue, Aug 21, 2018 at 12:48 PM, David Davis <daviddavis at redhat.com>
> wrote:
>
>> 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?
>>
>> David
>>
>>
>> On Thu, Aug 9, 2018 at 5:34 PM Jeff Ortel <jortel at redhat.com> wrote:
>>
>>>
>>>
>>> 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>
>>> 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/plugin/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
>>>>
>>>>
>>>
>>>
>>> _______________________________________________
>>> Pulp-dev mailing listPulp-dev at redhat.comhttps://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/20180828/ac27f62d/attachment.htm>