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/plugin/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
