[Pulp-dev] Revisit: sync modes

[Milan Kovacik]

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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/pulp-dev/attachments/20180809/19192cf5/attachment.htm>