Re: [Pulp-dev] Pulp smash test docs and issues

2018-08-09 Thread Daniel Alley
>
> I think this is fine where it is. pulp2 is going into maintenance mode at
> some point here soon.


That makes sense for the Pulp 2 smash test docs, but it's still a problem
if we want to have smash test docs for Pulp 3 (which, we do)

On Thu, Aug 9, 2018 at 5:12 PM, Brian Bouterse  wrote:

> Thanks for bringing this up. I put some responses inline.
>
> On Thu, Aug 9, 2018 at 4:22 PM, David Davis  wrote:
>
>> One of the things that may not make sense anymore is how we document and
>> track issues for pulp-smash tests given that these tests no longer reside
>> in the pulp-smash repo. Currently, all the test-related issues are tracked
>> here[0].
>>
>> With the tests no longer in the pulp-smash repo, I wonder if it makes
>> sense to maybe move them somewhere else like into redmine for Pulp 3 (or
>> the pulp-2-tests[1] repo for Pulp 2).
>>
> +1 to moving issues about the testing of a plugin to that plugin's tracker
>
>
>> The other question is about documentation. Currently the pulp-smash test
>> documentation is hosted on RTD (e.g. https://pulp-2-tests.rea
>> dthedocs.io/en/latest/). Should this documentation live alongside the
>> core/plugin docs?
>>
> I think this is fine where it is. pulp2 is going into maintenance mode at
> some point here soon.
>
>
>> QE would like to hear feedback as to how to proceed by August 20, 2018 so
>> please respond by then.
>>
>> Thanks.
>>
>> [0] https://github.com/pulpqe/pulp-smash/issues
>> [1] https://github.com/PulpQE/Pulp-2-Tests
>>
>> David
>>
>> ___
>> 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


Re: [Pulp-dev] Revisit: sync modes

2018-08-09 Thread Jeff Ortel



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 > 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 mailto:mkova...@redhat.com>> wrote:



On Wed, Aug 8, 2018 at 7:54 PM, Jeff Ortel mailto: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

Re: [Pulp-dev] Pulp smash test docs and issues

2018-08-09 Thread Brian Bouterse
Thanks for bringing this up. I put some responses inline.

On Thu, Aug 9, 2018 at 4:22 PM, David Davis  wrote:

> One of the things that may not make sense anymore is how we document and
> track issues for pulp-smash tests given that these tests no longer reside
> in the pulp-smash repo. Currently, all the test-related issues are tracked
> here[0].
>
> With the tests no longer in the pulp-smash repo, I wonder if it makes
> sense to maybe move them somewhere else like into redmine for Pulp 3 (or
> the pulp-2-tests[1] repo for Pulp 2).
>
+1 to moving issues about the testing of a plugin to that plugin's tracker


> The other question is about documentation. Currently the pulp-smash test
> documentation is hosted on RTD (e.g. https://pulp-2-tests.
> readthedocs.io/en/latest/). Should this documentation live alongside the
> core/plugin docs?
>
I think this is fine where it is. pulp2 is going into maintenance mode at
some point here soon.


> QE would like to hear feedback as to how to proceed by August 20, 2018 so
> please respond by then.
>
> Thanks.
>
> [0] https://github.com/pulpqe/pulp-smash/issues
> [1] https://github.com/PulpQE/Pulp-2-Tests
>
> David
>
> ___
> 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


Re: [Pulp-dev] Revisit: sync modes

2018-08-09 Thread Milan Kovacik
On Thu, Aug 9, 2018 at 8: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*
>>
>
> 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 
> 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 
>> wrote:
>>
>>>
>>>
>>> On Wed, Aug 8, 2018 at 7:54 PM, Jeff Ortel  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 

[Pulp-dev] Pulp smash test docs and issues

2018-08-09 Thread David Davis
One of the things that may not make sense anymore is how we document and
track issues for pulp-smash tests given that these tests no longer reside
in the pulp-smash repo. Currently, all the test-related issues are tracked
here[0].

With the tests no longer in the pulp-smash repo, I wonder if it makes sense
to maybe move them somewhere else like into redmine for Pulp 3 (or the
pulp-2-tests[1] repo for Pulp 2).

The other question is about documentation. Currently the pulp-smash test
documentation is hosted on RTD (e.g.
https://pulp-2-tests.readthedocs.io/en/latest/). Should this documentation
live alongside the core/plugin docs?

QE would like to hear feedback as to how to proceed by August 20, 2018 so
please respond by then.

Thanks.

[0] https://github.com/pulpqe/pulp-smash/issues
[1] https://github.com/PulpQE/Pulp-2-Tests

David
___
Pulp-dev mailing list
Pulp-dev@redhat.com
https://www.redhat.com/mailman/listinfo/pulp-dev


Re: [Pulp-dev] Revisit: sync modes

2018-08-09 Thread Daniel Alley
>
> 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?


On Thu, Aug 9, 2018 at 12:53 PM, Brian Bouterse  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  wrote:
>
>>
>>
>> On Wed, Aug 8, 2018 at 7:54 PM, Jeff Ortel  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?
>>>

Re: [Pulp-dev] Revisit: sync modes

2018-08-09 Thread Brian Bouterse
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  wrote:

>
>
> On Wed, Aug 8, 2018 at 7:54 PM, Jeff Ortel  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
>
>
___
Pulp-dev mailing list
Pulp-dev@redhat.com
https://www.redhat.com/mailman/listinfo/pulp-dev


Re: [Pulp-dev] Revisit: sync modes

2018-08-09 Thread Milan Kovacik
On Wed, Aug 8, 2018 at 7:54 PM, Jeff Ortel  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