Add alias "manualrun" for deprecated "disabledrun" #826
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
On 2020-07-24 14:24:59 -0700, Colleen Murphy wrote:
diff --git a/osc/commandline.py b/osc/commandline.py
index 9230589a..7efbe12b 100644
--- a/osc/commandline.py
+++ b/osc/commandline.py
@@ -6951,7 +6951,8 @@ def do_service(self, subcmd, opts, *args):
runall ra run all services independent of the used mode
localrun lr run all services except the ones with mode "buildtime", "disabled", or
"serveronly" (deprecated)
- disabledrun dr run all services with mode "disabled" or "serveronly" (deprecated)
+ manualrun mr run all services with mode "manual", "disabled", or "serveronly"
Hmm I first thought about "maintenance request" when I read "mr" but I
guess its meaning should be clear from the context:)
@adrianschroeter, @lethliel: any objections against a "manualrun"/"mr"
alias and a new "manual" mode?
+ disabledrun dr run all services with mode "disabled" or "serveronly" (deprecated, use "manualrun")
remoterun rr trigger a re-run on the server side
merge commits all server side generated files and drops the _service definition
wait waits until the service finishes and returns with an error if it failed
diff --git a/osc/core.py b/osc/core.py
index ee1cc16f..de646f94 100644
--- a/osc/core.py
+++ b/osc/core.py
@@ -453,9 +453,11 @@ def execute(self, dir, callmode = None, singleservice = None, verbose = None):
continue
if service['mode'] == "buildtime":
continue
- if service['mode'] == "serveronly" and callmode != "disabled":
+ if service['mode'] == "serveronly" and callmode not in ("manual", "disabled"):
continue
- if service['mode'] == "disabled" and callmode != "disabled":
+ if service['mode'] == "manual" and callmode not in ("manual", "disabled"):
+ continue
+ if service['mode'] == "disabled" and callmode not in ("manual", "disabled"):
continue
Just an idea: maybe we could make this a bit more concise - something like
manual_modes = ('manual', 'disabled')
...
if service['mode'] in manual_modes and callmode not in manual_modes:
continue
if service['mode'] != "disabled" and callmode == "disabled":
continue
On the one hand, this if-statement needs an adjustment, too (otherwise "dr"
and "mr" behave differently). Maybe something like
if service['mode'] not in manual_modes and callmode in manual_modes:
continue
On the other hand, this would effectively skip services with mode
"serveronly", which is a contradiction wrt. the docs (that is, our
old code was broken, too).
Hence, we should either fix the code or the docs:/
|
|
I am even thinking about to create "manual" as alias on the server side, I have to admit. The "disable" name was not very wisley picked from todays usage POV .... |
|
I tried to update the documentation and give some sense to the modes back: Also introducing "manual" as mode on the server side here: openSUSE/open-build-service#9959 Do the different modes make sense now? Maybe we can make a difference in future in osc, that "manualrun" is NOT executing the "disabled" ones, the two names have actually a meaning. What do you think about it? |
|
I would even change osc a little bit incompatible to give some sense back. For example this would be the help: I wonder if should run "disabled" services also with "manualrun" to give kind of backward compability for people who get used to the new command .... Does this make sense? |
|
@marcus-h @adrianschroeter I am fine with this. I would even remove localrun and disabledrun from the usage and just keep it in the Not for common usage anymore: section |
|
On Dienstag, 28. Juli 2020, 14:21:57 CEST wrote Marco Strigl:
@marcus-h @adrianschroeter I am fine with this.
I would even remove localrun and disabledrun from the usage and just keep it in the Not for common usage anymore: section
sounds good to me
…--
Adrian Schroeter
email: adrian@suse.de
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
Maxfeldstraße 5
90409 Nürnberg
Germany
|
|
@adrianschroeter commented on Jul 28, 2020, 1:23 AM PDT:
This is the way I proposed it, as just an alias, but I wonder now if that's really the best way forward, if you eventually want to change the meaning of "disabled" then it might be easier to keep them separate? I also wonder what to do about "serveronly", currently "disabledrun" also runs services with mode "serveronly" but the meaning of "serveronly" in the docs is completely contradictory to what I would use "disabledrun" for. Should "manualrun" also run "serveronly" or no? |
|
On 2020-07-28 10:10:37 -0700, Colleen Murphy wrote:
***@***.*****](https://github.com/adrianschroeter) commented on [Jul 28, 2020, 1:23 AM PDT](#826 (comment) "2020-07-28T08:23:43Z - Replied by Github Reply Comments"):
>
> I wonder if should run "disabled" services also with "manualrun" to give kind of backward compability for people who get used to the new command ....
>
> Does this make sense?
This is the way I proposed it, as just an alias, but I wonder now if that's really the best way forward, if you eventually want to change the meaning of "disabled" then it might be easier to keep them separate?
I also wonder what to do about "serveronly", currently "disabledrun" also runs services with mode "serveronly"
That's just what the docs claim:/
"disabledrun" skips services with mode "serveronly" (see also my comment
on the patch).
but the meaning of "serveronly" in the docs is completely contradictory to what I would use "disabledrun" for. Should "manualrun" also run "serveronly" or no?
IMHO, no. It seems that nobody expects that "disabledrun" also executes
"serveronly" services (at least nobody complained about skipping them).
|
|
On 2020-07-28 01:23:58 -0700, Adrian Schröter wrote:
I would even change osc a little bit incompatible to give some sense back. For example this would be the help:
<SNIP>
COMMAND can be:
run r run defined services locally, it takes an optional parameter to run only a
specified source service. In case parameters exist for this one in _service file
they are used.
Should we take the mode into account if the service is part of the
_service file?
remoterun rr trigger a re-run on the server side, no local change
runall ra run all services independent of the used mode
manualrun mr run all services with mode "manual"
merge commits all server side generated files and drops the _service definition
wait waits until the service finishes and returns with an error if it failed
Not for common usage anymore:
localrun lr run also the services using "serveronly" mode
Hmm this description is a bit sparse; Which modes are executed?
current: all modes except buildtime, serveronly, disabled
new: ?
Btw., is there a specific use case for the "serveronly" change?
disabledrun dr run all services with mode "disabled"
That's the status quo (except that the docs say something different).
To sum it up, the behavior of "localrun" will change (since it should
also execute services with mode "serveronly" + X). The behavior of
"run" will change (currently, parameters from the _service are ignored)
(maybe: also ignore the mode?).
I wonder if should run "disabled" services also with "manualrun" to give kind of backward compability for people who get used to the new command ....
Hmm...
First thought: I don't care, why not...
Second thought: No, no, no!:) We just do this new mode dance to add some
clarity to existing mode terminology + behavior. Executing "disabled"
services in case of "manualrun" would thwart that plan:) (IMHO)
Does this make sense?
Yes!
|
|
On Mittwoch, 29. Juli 2020, 02:18:40 CEST wrote Marcus Hüwe:
On 2020-07-28 01:23:58 -0700, Adrian Schröter wrote:
> I would even change osc a little bit incompatible to give some sense back. For example this would be the help:
<SNIP>
> COMMAND can be:
> run r run defined services locally, it takes an optional parameter to run only a
> specified source service. In case parameters exist for this one in _service file
> they are used.
Should we take the mode into account if the service is part of the
_service file?
yes. Maybe we need to describe it better like this:
This runs configured services locally. This includes the services executed on server side as well.
It can be used to check if configured service have the expected
results and it happens by default before each local build.
> remoterun rr trigger a re-run on the server side, no local change
> runall ra run all services independent of the used mode
> manualrun mr run all services with mode "manual"
> merge commits all server side generated files and drops the _service definition
> wait waits until the service finishes and returns with an error if it failed
>
> Not for common usage anymore:
> localrun lr run also the services using "serveronly" mode
Hmm this description is a bit sparse; Which modes are executed?
current: all modes except buildtime, serveronly, disabled
new: ?
Btw., is there a specific use case for the "serveronly" change?
It is basically the same as "run" plus serveronly services.
Only for very rare setups where services are configured "serveronly" and
this is mostly only used when the service needs further setup in a
way that it can be executed locally.
So it is only for the person who wants to debug/develop the "serveronly"
configured service (eg. a virus scanner needed access to a private
database host for example).
> disabledrun dr run all services with mode "disabled"
>
That's the status quo (except that the docs say something different).
except removing the execution of "serveronly", which IMHO makes no
sense here. But again, only very few people have configured "serveronly"
services afaik. (less then 10 in total?)
To sum it up, the behavior of "localrun" will change (since it should
also execute services with mode "serveronly" + X).
That is actually no change, it is the current situation.
The behavior of
"run" will change (currently, parameters from the _service are ignored)
(maybe: also ignore the mode?).
That one is not supposed to change. All, but "disabled", "manual" and "serveronly"
are executed.
> I wonder if should run "disabled" services also with "manualrun" to give kind of backward compability for people who get used to the new command ....
>
Hmm...
First thought: I don't care, why not...
Second thought: No, no, no!:) We just do this new mode dance to add some
clarity to existing mode terminology + behavior. Executing "disabled"
services in case of "manualrun" would thwart that plan:) (IMHO)
yes, I agree absolute.
The only problem I see is that someone needs to check the _service file
first to know if he needs to execute "manualrun" or "disabledrun".
However, maybe not a problem... or if it is, we can still do something
afterwards.
> Does this make sense?
>
Yes!
Great :)
…--
Adrian Schroeter
email: adrian@suse.de
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
Maxfeldstraße 5
90409 Nürnberg
Germany
|
|
On Dienstag, 28. Juli 2020, 20:40:55 CEST wrote Marcus Hüwe:
On 2020-07-28 10:10:37 -0700, Colleen Murphy wrote:
> ***@***.*****](https://github.com/adrianschroeter) commented on [Jul 28, 2020, 1:23 AM PDT](#826 (comment) "2020-07-28T08:23:43Z - Replied by Github Reply Comments"):
>
> >
> > I wonder if should run "disabled" services also with "manualrun" to give kind of backward compability for people who get used to the new command ....
> >
> > Does this make sense?
>
> This is the way I proposed it, as just an alias, but I wonder now if that's really the best way forward, if you eventually want to change the meaning of "disabled" then it might be easier to keep them separate?
>
> I also wonder what to do about "serveronly", currently "disabledrun" also runs services with mode "serveronly"
That's just what the docs claim:/
"disabledrun" skips services with mode "serveronly" (see also my comment
on the patch).
ah :)
> but the meaning of "serveronly" in the docs is completely contradictory to what I would use "disabledrun" for. Should "manualrun" also run "serveronly" or no?
>
IMHO, no. It seems that nobody expects that "disabledrun" also executes
"serveronly" services (at least nobody complained about skipping them).
Right, that should be cleaned up.
…--
Adrian Schroeter
email: adrian@suse.de
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
Maxfeldstraße 5
90409 Nürnberg
Germany
|
|
On 2020-07-28 22:59:08 -0700, Adrian Schröter wrote:
On Mittwoch, 29. Juli 2020, 02:18:40 CEST wrote Marcus Hüwe:
> On 2020-07-28 01:23:58 -0700, Adrian Schröter wrote:
> > I would even change osc a little bit incompatible to give some sense back. For example this would be the help:
>
> <SNIP>
>
> > COMMAND can be:
> > run r run defined services locally, it takes an optional parameter to run only a
> > specified source service. In case parameters exist for this one in _service file
> > they are used.
>
> Should we take the mode into account if the service is part of the
> _service file?
yes.
I was talking about "osc service run foo" where "foo" is a serveronly
service (sorry for the confusion!). Should "foo" be executed?
Maybe we need to describe it better like this:
This runs configured services locally. This includes the services executed on server side as well.
Huh? This reads as if "serveronly" services are executed as well.
current: trylocal, localonly, no mode
It can be used to check if configured service have the expected
results and it happens by default before each local build.
Sounds good!
> > remoterun rr trigger a re-run on the server side, no local change
> > runall ra run all services independent of the used mode
> > manualrun mr run all services with mode "manual"
> > merge commits all server side generated files and drops the _service definition
> > wait waits until the service finishes and returns with an error if it failed
> >
> > Not for common usage anymore:
> > localrun lr run also the services using "serveronly" mode
>
> Hmm this description is a bit sparse; Which modes are executed?
> current: all modes except buildtime, serveronly, disabled
> new: ?
>
> Btw., is there a specific use case for the "serveronly" change?
It is basically the same as "run" plus serveronly services.
Ah ok.
Only for very rare setups where services are configured "serveronly" and
this is mostly only used when the service needs further setup in a
way that it can be executed locally.
So it is only for the person who wants to debug/develop the "serveronly"
configured service (eg. a virus scanner needed access to a private
database host for example).
> > disabledrun dr run all services with mode "disabled"
> >
> That's the status quo (except that the docs say something different).
except removing the execution of "serveronly", which IMHO makes no
sense here. But again, only very few people have configured "serveronly"
services afaik. (less then 10 in total?)
> To sum it up, the behavior of "localrun" will change (since it should
> also execute services with mode "serveronly" + X).
That is actually no change, it is the current situation.
No.
current: trylocal, localonly, no mode
(that is, the same as "run")
> The behavior of
> "run" will change (currently, parameters from the _service are ignored)
> (maybe: also ignore the mode?).
That one is not supposed to change. All, but "disabled", "manual" and "serveronly"
are executed.
+ buildtime
(I was just referring to the fact that in future we also consider parameters
from the _service file in the singleservice ("osc service run foo") case (if
"foo" is part of the _service file).)
> > I wonder if should run "disabled" services also with "manualrun" to give kind of backward compability for people who get used to the new command ....
> >
> Hmm...
> First thought: I don't care, why not...
> Second thought: No, no, no!:) We just do this new mode dance to add some
> clarity to existing mode terminology + behavior. Executing "disabled"
> services in case of "manualrun" would thwart that plan:) (IMHO)
yes, I agree absolute.
The only problem I see is that someone needs to check the _service file
first to know if he needs to execute "manualrun" or "disabledrun".
Good point.
However, maybe not a problem... or if it is, we can still do something
afterwards.
Ok. Let's see:)
|
cmurphy
force-pushed
the
undeprecate-disabledrun
branch
from
8883474
to
0544251
Compare
|
@marcus-h commented on Jul 29, 2020, 4:15 AM PDT:
[snip]
I don't think this patch should change the behavior of a singleservice run. The current behavior is the mode is read from the _service file but the parameters are ignored. I have a different change proposed to fix that behavior #769 (it will need to be reworked depending on this change). I'm updating this patch to run "serveronly" services for call mode "local" which I think is in line with the documentation, and changing "manualrun" to only trigger "manual" services and leaving "disabledrun" to continue only triggering "disabled" services but no longer "serveronly" services since that wasn't working, and also updated the command documentation. I think that captures the feedback given here but let me know if I've missed something. |
|
On 2020-07-29 11:23:13 -0700, Colleen Murphy wrote:
***@***.*****](https://github.com/marcus-h) commented on [Jul 29, 2020, 4:15 AM PDT](#826 (comment) "2020-07-29T11:15:07Z - Replied by Github Reply Comments"):
> On 2020-07-28 22:59:08 -0700, Adrian Schröter wrote:
> > On Mittwoch, 29. Juli 2020, 02:18:40 CEST wrote Marcus Hüwe:
[snip]
> > > The behavior of
> > > "run" will change (currently, parameters from the _service are ignored)
> > > (maybe: also ignore the mode?).
> >
> > That one is not supposed to change. All, but "disabled", "manual" and "serveronly"
> > are executed.
> >
> + buildtime
> (I was just referring to the fact that in future we also consider parameters
> from the _service file in the singleservice ("osc service run foo") case (if
> "foo" is part of the _service file).)
I don't think this patch should change the behavior of a singleservice run. The current behavior is the mode is read from the _service file but the parameters are ignored.
No, the mode is also ignored (since "not singleservice in allservices"
always holds (which is, as you already pointed out, a bug:) ). But
I'm perfectly fine to postpone this:)
I have a different change proposed to fix that behavior #769 (it will need to be reworked depending on this change).
I'm updating this patch to run "serveronly" services for call mode "local" which I think is in line with the documentation, and changing "manualrun" to only trigger "manual" services and leaving "disabledrun" to continue only triggering "disabled" services but no longer "serveronly" services since that wasn't working, and also updated the command documentation. I think that captures the feedback given here but let me know if I've missed something.
That sounds good.
Maybe a short (hopefully correct) "summary":
callmode=None (run): trylocal, localonly, no mode
callmode=all (runall): buildtime, serveronly, disabled, trylocal,
localonly, manual, no mode
callmode=local (localrun): serveronly, trylocal, localonly, no mode
callmode=disabled (disabledrun): disabled
callmode=manual (manualrun): manual
(where "no mode" corresponds to an omitted "mode" attribute in the
_service file)
@adrianschroeter: does the summary make sense?
Thanks a lot for working on this! Much appreciated! (Especially after
this lengthy discussion(s) :) )
Once you are done with the patch, just add a small comment to this
PR and we will happily review it:)
|
|
On Donnerstag, 30. Juli 2020, 00:10:16 CEST wrote Marcus Hüwe:
On 2020-07-29 11:23:13 -0700, Colleen Murphy wrote:
> ***@***.*****](https://github.com/marcus-h) commented on [Jul 29, 2020, 4:15 AM PDT](#826 (comment) "2020-07-29T11:15:07Z - Replied by Github Reply Comments"):
> > On 2020-07-28 22:59:08 -0700, Adrian Schröter wrote:
> > > On Mittwoch, 29. Juli 2020, 02:18:40 CEST wrote Marcus Hüwe:
>
> [snip]
>
> > > > The behavior of
> > > > "run" will change (currently, parameters from the _service are ignored)
> > > > (maybe: also ignore the mode?).
> > >
> > > That one is not supposed to change. All, but "disabled", "manual" and "serveronly"
> > > are executed.
> > >
> > + buildtime
> > (I was just referring to the fact that in future we also consider parameters
> > from the _service file in the singleservice ("osc service run foo") case (if
> > "foo" is part of the _service file).)
>
> I don't think this patch should change the behavior of a singleservice run. The current behavior is the mode is read from the _service file but the parameters are ignored.
No, the mode is also ignored (since "not singleservice in allservices"
always holds (which is, as you already pointed out, a bug:) ). But
I'm perfectly fine to postpone this:)
> I have a different change proposed to fix that behavior #769 (it will need to be reworked depending on this change).
>
> I'm updating this patch to run "serveronly" services for call mode "local" which I think is in line with the documentation, and changing "manualrun" to only trigger "manual" services and leaving "disabledrun" to continue only triggering "disabled" services but no longer "serveronly" services since that wasn't working, and also updated the command documentation. I think that captures the feedback given here but let me know if I've missed something.
>
That sounds good.
Maybe a short (hopefully correct) "summary":
callmode=None (run): trylocal, localonly, no mode
callmode=all (runall): buildtime, serveronly, disabled, trylocal,
localonly, manual, no mode
maybe we should not execute "disabled" here anymore. Would give
this mode the original purpose back. Yes, incompatible change,
but gives sense to the two disabled and manual modes ...
callmode=local (localrun): serveronly, trylocal, localonly, no mode
callmode=disabled (disabledrun): disabled
callmode=manual (manualrun): manual
(where "no mode" corresponds to an omitted "mode" attribute in the
_service file)
@adrianschroeter: does the summary make sense?
yes, it does for me!
Thanks a lot for working on this! Much appreciated! (Especially after
this lengthy discussion(s) :) )
Indeed, do not give up with us please! :)
…--
Adrian Schroeter
email: adrian@suse.de
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
Maxfeldstraße 5
90409 Nürnberg
Germany
|
|
On 2020-07-29 23:45:39 -0700, Adrian Schröter wrote:
On Donnerstag, 30. Juli 2020, 00:10:16 CEST wrote Marcus Hüwe:
<SNIP>
> Maybe a short (hopefully correct) "summary":
>
> callmode=None (run): trylocal, localonly, no mode
> callmode=all (runall): buildtime, serveronly, disabled, trylocal,
> localonly, manual, no mode
maybe we should not execute "disabled" here anymore. Would give
this mode the original purpose back. Yes, incompatible change,
but gives sense to the two disabled and manual modes ...
Hmm sounds more like a "runenabled":P IMHO, all should really mean
"all". If I understand it correctly, you are proposing
callmode=all (runall): buildtime, serveronly, trylocal, localonly,
manual, no mode
(all modes except "disabled"), right?
That is, "manual" can never be used as a replacement for "disabled".
(I can perfectly live with your proposal, but then we should still
encourage the usage of "disabledrun" in "osc service --help".)
… > callmode=local (localrun): serveronly, trylocal, localonly, no mode
> callmode=disabled (disabledrun): disabled
> callmode=manual (manualrun): manual
>
> (where "no mode" corresponds to an omitted "mode" attribute in the
> _service file)
|
|
@marcus-h commented on Jul 29, 2020, 3:10 PM PDT:
You're right, I got mixed up with the other patch which changes things, will worry about that later.
:)
From my perspective this is ready to review, unless there is agreement that the runall command needs to be changed? I think it would be better to leave it as it is. |
|
On 2020-07-30 10:20:14 -0700, Colleen Murphy wrote:
***@***.*****](https://github.com/marcus-h) commented on [Jul 29, 2020, 3:10 PM PDT](#826 (comment) "2020-07-29T22:10:01Z - Replied by Github Reply Comments"):
> Once you are done with the patch, just add a small comment to this
> PR and we will happily review it:)
From my perspective this is ready to review, unless there is agreement that the runall command needs to be changed? I think it would be better to leave it as it is.
Ok. LGTM except a small "bug" (see below) (feel free to ignore the
other nitpicks and suggestions).
Subject: [PATCH] Add command "manualrun" to replace "disabledrun"
The "disabledrun" service commands is marked as deprecated but has no
explicit replacement. It is still a useful command for updating packages
manually or through a CI system without being forced to run all defined
services with the "runall" command. This change adds a new command
"manualrun" and a new mode "manual" which behave the same as the
deprecated "disabledrun" command and "disabled" mode but have clearer
meaning. "manualrun" does not attempt backwards-compatible behavior with
the "disabledrun" mode for "disabled" services because "disabled" mode
may eventually be removed or change meaning. This change also corrects
the behavior of "serveronly" mode to respect the "localrun" command in
order to be in line with the documented behavior.
Nitpick: "localrun" is "enhanced" in order to consider the
"serveronly" mode. Since "disabledrun" never executed services with mode
"serveronly", its docs are updated accordingly.
---
osc/commandline.py | 14 +++++++++-----
osc/core.py | 6 ++++--
2 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/osc/commandline.py b/osc/commandline.py
index 9230589a..3f13cf50 100644
--- a/osc/commandline.py
+++ b/osc/commandline.py
@@ -6949,13 +6949,15 @@ def do_service(self, subcmd, opts, *args):
specified source service. In case parameters exist for this one in _service file
they are used.
runall ra run all services independent of the used mode
- localrun lr run all services except the ones with mode "buildtime", "disabled", or
- "serveronly" (deprecated)
- disabledrun dr run all services with mode "disabled" or "serveronly" (deprecated)
+ manualrun mr run all services with mode "manual"
remoterun rr trigger a re-run on the server side
merge commits all server side generated files and drops the _service definition
wait waits until the service finishes and returns with an error if it failed
+ Not for common usage anymore:
+ localrun lr run also the services using "serveronly" mode
Maybe: 'the same as "run" but services with mode "serveronly" are also
executed'?
+ disabledrun dr run all services with mode "disabled"
+
<SNIP>
diff --git a/osc/core.py b/osc/core.py
index ee1cc16f..c5fa128b 100644
--- a/osc/core.py
+++ b/osc/core.py
@@ -453,7 +453,9 @@ def execute(self, dir, callmode = None, singleservice = None, verbose = None):
continue
if service['mode'] == "buildtime":
continue
- if service['mode'] == "serveronly" and callmode != "disabled":
+ if service['mode'] == "serveronly" and callmode != "local":
+ continue
+ if service['mode'] == "manual" and callmode != "manual":
continue
Currently, "osc service manualrun" would also execute services with mode
trylocal, localonly, and no mode. Adding something like
if service['mode'] != 'manual' and callmode == 'manual':
continue
should do the trick.
Frankly speaking, we should rewrite this mode checking logic in the
future... Something like
if callmode is None:
if service['mode'] not in ('trylocal', 'localonly', ''):
continue
elif callmode == 'local':
if service['mode'] not in ('serveronly', 'trylocal', 'localonly', ''):
continue
elif callmode == 'disabled':
if service['mode'] != 'disabled':
continue
elif callmode == 'manual':
if service['mode'] != 'manual':
continue
elif callmode != 'all':
# unexpected callmode (reconsider the existing callmode == 'trylocal'
# behavior)
...
is much more readable (IMHO). But that's probably something for a
another PR:)
|
The "disabledrun" service commands is marked as deprecated but has no explicit replacement. It is still a useful command for updating packages manually or through a CI system without being forced to run all defined services with the "runall" command. This change adds a new command "manualrun" and a new mode "manual" which behave the same as the deprecated "disabledrun" command and "disabled" mode but have clearer meaning. "manualrun" does not attempt backwards-compatible behavior with the "disabledrun" mode for "disabled" services because "disabled" mode may eventually be removed or change meaning. The "localrun" command is enhanced to consider the "serveronly" mode. Since "disabledrun" never executed services with mode "serveronly", its docs are updated accordingly.
cmurphy
force-pushed
the
undeprecate-disabledrun
branch
from
0544251
to
894b97d
Compare
|
@marcus-h commented on Jul 30, 2020, 1:34 PM PDT:
done
done
Good catch, thanks
I will let someone else wade into those weeds :) |
|
On 2020-07-30 14:30:42 -0700, Colleen Murphy wrote:
***@***.*****](https://github.com/marcus-h) commented on [Jul 30, 2020, 1:34 PM PDT](#826 (comment) "2020-07-30T20:34:57Z - Replied by Github Reply Comments"):
LGTM. Thanks a lot! If @adrianschroeter is fine with keeping the
current "runall" semantics, I'll merge it later today (where no
comment implies being fine:) ).
> Frankly speaking, we should rewrite this mode checking logic in the
> future...
I will let someone else wade into those weeds :)
Well such "obvious" cleanups usually do not require many rounds of
review... just saying:P
|
|
On Freitag, 31. Juli 2020, 00:18:28 CEST wrote Marcus Hüwe:
On 2020-07-30 14:30:42 -0700, Colleen Murphy wrote:
> ***@***.*****](https://github.com/marcus-h) commented on [Jul 30, 2020, 1:34 PM PDT](#826 (comment) "2020-07-30T20:34:57Z - Replied by Github Reply Comments"):
>
LGTM. Thanks a lot! If @adrianschroeter is fine with keeping the
current "runall" semantics, I'll merge it later today (where no
comment implies being fine:) ).
I am still in favour changing it, but we can do this kind of incompatible
step also later...
…--
Adrian Schroeter
email: adrian@suse.de
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
Maxfeldstraße 5
90409 Nürnberg
Germany
|
|
On 2020-07-30 23:15:34 -0700, Adrian Schröter wrote:
On Freitag, 31. Juli 2020, 00:18:28 CEST wrote Marcus Hüwe:
> On 2020-07-30 14:30:42 -0700, Colleen Murphy wrote:
> > ***@***.*****](https://github.com/marcus-h) commented on [Jul 30, 2020, 1:34 PM PDT](#826 (comment) "2020-07-30T20:34:57Z - Replied by Github Reply Comments"):
> >
>
> LGTM. Thanks a lot! If @adrianschroeter is fine with keeping the
> current "runall" semantics, I'll merge it later today (where no
> comment implies being fine:) ).
I am still in favour changing it, but we can do this kind of incompatible
step also later...
Ok. Merged for now.
Again, thanks a lot!
|
dirkmueller
added a commit
to dirkmueller/obs-service-source_validator
that referenced
this pull request
Mar 21, 2022
manual is the new name of disabled, see openSUSE/osc#826 Stop advertising deprecated names, so shorten warning message. Remove explicit as the api does not allow that: https://github.com/openSUSE/open-build-service/blob/4a4d5b0f07dd0d6bc03def3e863d299bfebcd7ad/docs/api/api/obs.rng#L88-L96
dirkmueller
added a commit
to dirkmueller/obs-service-source_validator
that referenced
this pull request
Aug 3, 2022
manual is the new name of disabled, see openSUSE/osc#826 Stop advertising deprecated names, so shorten warning message. Remove explicit as the api does not allow that: https://github.com/openSUSE/open-build-service/blob/4a4d5b0f07dd0d6bc03def3e863d299bfebcd7ad/docs/api/api/obs.rng#L88-L96
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The "disabledrun" service commands is marked as deprecated but has no
explicit replacement. It is still a useful command for updating packages
manually or through a CI system without being forced to run all defined
services with the "runall" command. This change adds an alias
"manualrun" and a new mode "manual" which behave the same as the
deprecated "disabledrun" command and "disabled" mode but have clearer
meaning.
Followup from discussion on #769