On Thu, Aug 9, 2018 at 8:29 PM, Daniel Alley <dal...@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 <bbout...@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 <mkova...@redhat.com> >> wrote: >> >>> >>> >>> On Wed, Aug 8, 2018 at 7:54 PM, Jeff Ortel <jor...@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@redhat.com >>>> https://www.redhat.com/mailman/listinfo/pulp-dev >>>> >>>> >>> >>> _______________________________________________ >>> Pulp-dev mailing list >>> Pulp-dev@redhat.com >>> https://www.redhat.com/mailman/listinfo/pulp-dev >>> >>> >> >> _______________________________________________ >> Pulp-dev mailing list >> Pulp-dev@redhat.com >> https://www.redhat.com/mailman/listinfo/pulp-dev >> >> >
_______________________________________________ Pulp-dev mailing list Pulp-dev@redhat.com https://www.redhat.com/mailman/listinfo/pulp-dev
