add support for new staging api. (command osc ps) #631

lethliel · 2019-09-12T10:17:11Z

Create, Add, Delete staging projects
Add requests to staging

It can be issued within a project directory or with the
project as the first argument.

If you operate on a staging project or with requests the
staging project has to be given with -s and the request has
to be given with -r

You can add multiple requests / stagings comma serparated.
For example:

-s myprj:Staging:A, myprj:Staging:B
 -r 1234, 12345

osc ps

For example:
linux:~/myproject/> osc ps create
or
osc ps create myproject

STILL IN BETA

This fixes #583

lethliel · 2019-09-12T10:20:42Z

@marcus-h: This is a beta version of the new staging command. Can we please merge this, so that the release engineers have something to test and do the refactoring aftwerwards?

Thanks and cheers,
Marco

vpereira · 2019-09-12T10:51:46Z

@coolo

marcus-h · 2019-09-12T11:23:16Z

Just a very, very quick and high-level review:

On 2019-09-12 03:17:13 -0700, Marco Strigl wrote: diff --git a/osc/commandline.py b/osc/commandline.py index 2eea1e8e..3e1e538e 100644 --- a/osc/commandline.py +++ b/osc/commandline.py @@ -9245,6 +9245,225 @@ def do_comment(self, subcmd, opts, *args): result = delete_comment(apiurl, args[1]) print(result) + @cmdln.option('-g', '--group', + help='group', metavar='GROUP') + @cmdln.option('-s', '--staging', + help='staging project', metavar='STAGING')

Hmm what about @cmdln.option('-s', '--staging', action='append', default=[], ...) Then, we could use "-s foo -s bar -s xyz" instead of "-s foo,bar,xyz".

+ @cmdln.option('-r', '--requests', + help='request(s)', metavar='REQUESTS')

As above.

+ @cmdln.option('-v', '--verbose', action='store_true', + help='print extra information') + @cmdln.alias('ps') + def do_projectstaging(self, subcmd, opts, *args):

We could do def do_projectstaging(self, subcmd, opts, cmd, *args): to get rid of the initial args[0] handling (use cmd instead).

+ """${cmd_name}: Handle staging + + Create, Add, Delete staging projects + Add requests to staging + + This can be issued within a project directory or with the + project as the first argument. + + If you operate on a staging project or with requests the + staging project has to be given with -s and the request has + to be given with -r + + You can add multiple requests / stagings comma serparated.

Typo: separated

+ For example: + -s myprj:Staging:A, myprj:Staging:B + -r 1234, 12345

Both arguments should be quoted "-s myprj:Staging:A, myprj:Staging:B". Note that the whitespace would be part of the project name (I'm not sure if a user would expect this). Maybe it is a bit more "natural" to specify the option several times (see above).

+ + osc ps + + For example: + linux:~/myproject/> osc ps create + or + osc ps create myproject + + subcommands: + create Create a staging workflow for the project + add Add a staging project to a staging workflow + using -s <staging project> + list List all staging project associated to this + workflow + show Show information to one specific staging project + using -s <staging project> + delete + Delete complete staging workflow from project + accept + Accept the staging given with -s. All requests in + the staging project will be accepted + + + osc ps request + + subcommands: + add Add requests given with -r to a staging given with -s + delete Delete requests given with -r from a staging given with -s + list List all requests in a staging given with -s + + + + Example Workflow: + osc ps create myprj -g group_for_review + osc ps add myprj -s myprj:Staging:A + osc ps request add myprj -s myprj:Staging:A -r 1234, 4567 + osc ps request delete myprj -s myprj:Staging:A -r 1234 + osc ps request list -s myprj:Staging:A + osc ps accept myprj -s myprj:Staging:A + osc ps show myprj -s myprj:Staging:A + + + """ + + args = slash_split(args) + + subcmdlist = ['create', 'add', 'delete', 'show', 'request', 'rq', 'list', 'accept'] + rqcmdlist = ['add', 'delete', 'list'] + + if not args or args[0] not in subcmdlist: + raise oscerr.WrongArgs('Unknown operation. Choose one of %s.' \ + % ', '.join(subcmdlist)) + + has_staging_opt = None + if opts.staging: + staging_list = [] + st = opts.staging.split(',') + staging_list.extend(st)

What about staging_list = opts.staging.split(',')

+ has_staging_opt = True +

Hmm instead of has_staging_opt, we could use staging_list (just define "staging_list = []" before the if statement). <SNIP>

+ if cmd == 'create': + if not opts.group: + raise oscerr.WrongArgs('Please specify a group for the staging with -g <groupname>') + ret = ps.create(project, opts.group) + root = ET.fromstring(ret.read()) + summary = root.find('summary') + if summary.text == 'Ok': + print('Staging workflow for %s created!' % project) + create_sub = input('Do you want to automatically create staging projects? [Y|n]: ') or 'y'

raw_input. <SNIP>

+ elif cmd == 'delete': + res = ps.list(project) + root = ET.parse(res).getroot() + staging_list = [] + for stage in root.findall('staging_project'): + staging_list.append(stage.get('name')) + ret = ps.delete(project) + if ret: + print('Staging workflow for %s deleted!' % project) + if len(staging_list) >= 1:

Hmm simply: if staging_list:

+ print('Staging projects:') + for stage in staging_list: + print(stage) + delete_subs = input('Do you want to delete all staging projects for this workflow? [Y|n]: ') or 'y'

raw_input

+ if delete_subs in ['y', 'Y']: + delete_msg = 'Deleted by staging workflow' + for stage in staging_list: + delete_project(apiurl, stage, msg=delete_msg) + elif cmd == 'show': + for st in opts.staging.split(','):

Minor: Iterate over staging_list? <SNIP>

+ elif cmd in ['rq', 'request']: + if rqcmd == 'add': + rqlist = [] + rq = opts.requests.split(',') + rqlist.extend(rq) + ret = ps.add_request(project, staging_list[0], rqlist) + root = ET.fromstring(ret.read()) + summary = root.find('summary') + if summary.text == 'Ok': + for rq in rqlist: + print('Request %s added to %s' % (rq, staging_list[0]))

Hmm what about the other potential items in staging_list? (Note: I have no clue about the staging workflow... so the code is probably right...).

+ elif rqcmd == 'list': + for st in staging_list: + print('Requests in %s', st) + ret = ps.list_requests(project, st) + root = ET.fromstring(ret.read()) + for rq in root.findall('request'): + rq_id = rq.get('id') + creator = rq.get('creator') + state = rq.get('state') + pkg = rq.get('package') + print('%s\t\t%s\t\t%s\t\t%s' % (rq_id, creator, state, pkg)) + elif rqcmd == 'delete': + rqlist = [] + rq = opts.requests.split(',') + rqlist.extend(rq) + ret = ps.delete_requests(project, staging_list[0], rqlist) + root = ET.fromstring(ret.read()) + summary = root.find('summary') + if summary.text == 'Ok': + for rq in rqlist: + print('Request %s deleted from %s' % (rq, staging_list[0]))

Same question as in the "rqcmd == 'add'" case.

diff --git a/osc/core.py b/osc/core.py index b7d097db..2500da99 100644 --- a/osc/core.py +++ b/osc/core.py @@ -484,6 +484,79 @@ def execute(self, dir, callmode = None, singleservice = None, verbose = None): return 0 +class ProjectStaging: + """ProjectStaging actions and information + """

<SNIP>

+ def list(self, project): + try: + u = makeurl(self.apiurl, ['staging', project, 'staging_projects']) + f = http_GET(u) + except HTTPError as e: + if e.code == 400: + return None

Hmm in which case does the API return a 400?

+ raise e + return f + + def add(self, project, staging_list): + stagingxml = "<workflow>" + for stage in staging_list: + stagingxml += ("<staging_project>%s</staging_project>" % stage)

Unnecessary parentheses.

marcus-h · 2019-09-12T11:30:20Z

On 2019-09-12 03:20:43 -0700, Marco Strigl wrote: @marcus-h: This is a beta version of the new staging command. Can we please merge this, so that the release engineers have something to test and do the refactoring aftwerwards?

Hmm I somehow fear that this will eventually result in a similar "bloated" command (code-wise) as do_request:/ If a refactoring would "block" people, I'm fine with merging it [1], but let's at least do the "input" -> "raw_input" change and let's try to "fix" the UI ("-s foo,bar" vs. "-s foo -s bar"). [1] at least I have no objections from a high-level POV

lethliel · 2019-09-12T12:13:10Z

I have done all your requested changes. You are right. Setting the option multiple times is better. * switched to raw_input * used cmd in function call *

On 9/12/19 1:23 PM, Marcus Hüwe wrote: Just a very, very quick and high-level review:

<SNIP>

Hmm what about the other potential items in staging_list? (Note: I have no clue about the staging workflow... so the code is probably right...). > + elif rqcmd == 'list': > + for st in staging_list: > + print('Requests in %s', st) > + ret = ps.list_requests(project, st) > + root = ET.fromstring(ret.read()) > + for rq in root.findall('request'): > + rq_id = rq.get('id') > + creator = rq.get('creator') > + state = rq.get('state') > + pkg = rq.get('package') > + print('%s\t\t%s\t\t%s\t\t%s' % (rq_id, creator, state, pkg)) > + elif rqcmd == 'delete': > + rqlist = [] > + rq = opts.requests.split(',') > + rqlist.extend(rq) > + ret = ps.delete_requests(project, staging_list[0], rqlist) > + root = ET.fromstring(ret.read()) > + summary = root.find('summary') > + if summary.text == 'Ok': > + for rq in rqlist: > + print('Request %s deleted from %s' % (rq, staging_list[0])) Same question as in the "rqcmd == 'add'" case.

Becasue a request can only be added to one staging. Also it can only be deleted from one staging.

> diff --git a/osc/core.py b/osc/core.py > index b7d097db..2500da99 100644 > --- a/osc/core.py > +++ b/osc/core.py > @@ -484,6 +484,79 @@ def execute(self, dir, callmode = None, singleservice = None, verbose = None): > > return 0 > > +class ProjectStaging: > + """ProjectStaging actions and information > + """ <SNIP> > + def list(self, project): > + try: > + u = makeurl(self.apiurl, ['staging', project, 'staging_projects']) > + f = http_GET(u) > + except HTTPError as e: > + if e.code == 400: > + return None Hmm in which case does the API return a 400?

If you want to get a list of staging projects for a staging project that is not created. There is also a bug for this in open-build-service: openSUSE/open-build-service#8298

> + raise e > + return f > + > + def add(self, project, staging_list): > + stagingxml = "<workflow>" > + for stage in staging_list: > + stagingxml += ("<staging_project>%s</staging_project>" % stage) Unnecessary parentheses.

removed.

coolo · 2019-09-17T12:11:15Z

coolo@f102#osc>osc ps
Traceback (most recent call last):
  File "/suse/coolo/prod/osc/osc-wrapper.py", line 41, in <module>
    r = babysitter.run(osccli)
  File "/space/prod/osc/osc/babysitter.py", line 62, in run
    return prg.main(argv)
  File "/space/prod/osc/osc/cmdln.py", line 344, in main
    return self.cmd(args)
  File "/space/prod/osc/osc/cmdln.py", line 367, in cmd
    retval = self.onecmd(argv)
  File "/space/prod/osc/osc/cmdln.py", line 501, in onecmd
    return self._dispatch_cmd(handler, argv)
  File "/space/prod/osc/osc/cmdln.py", line 1232, in _dispatch_cmd
    return handler(argv[0], opts, *args)
TypeError: do_projectstaging() missing 1 required positional argument: 'cmd'

coolo · 2019-09-17T12:13:37Z

why do I need to give the name of the staging project to remove from?

lethliel · 2019-09-17T12:23:54Z

why do I need to give the name of the staging project to remove from?

Because the http_request to remove a sr is:

DELETE staging/home:mstrigl/staging_projects/Staging:A/staged_requests

and expects a xml with the requests to delete. osc does not know which request is in
which staging.

I would have to iterate over all stagings and search for the request to build the url

coolo · 2019-09-17T12:25:43Z

So you let the user do it instead?

coolo · 2019-09-17T12:26:05Z

But if that's the API, the API sucks - easy as that :)

lethliel · 2019-09-17T12:29:29Z

So you let the user do it instead?

Just explaining why I did it this way :-) I will implement that osc determines the staging for the request when no staging is given with -s

coolo · 2019-09-17T12:47:38Z

I think the API should handle this, really.

Create, Add, Delete staging projects Add requests to staging It can be issued within a project directory or with the project as the first argument. If you operate on a staging project or with requests the staging project has to be given with -s and the request has to be given with -r You can add multiple requests / stagings. For example: "-s myprj:Staging:A -s myprj:Staging:B" "-r 1234 -r 12345" osc ps For example: linux:~/myproject/> osc ps create or osc ps create myproject

lethliel · 2019-09-18T08:17:45Z

coolo@f102#osc>osc ps
Traceback (most recent call last):
  File "/suse/coolo/prod/osc/osc-wrapper.py", line 41, in <module>
    r = babysitter.run(osccli)
  File "/space/prod/osc/osc/babysitter.py", line 62, in run
    return prg.main(argv)
  File "/space/prod/osc/osc/cmdln.py", line 344, in main
    return self.cmd(args)
  File "/space/prod/osc/osc/cmdln.py", line 367, in cmd
    retval = self.onecmd(argv)
  File "/space/prod/osc/osc/cmdln.py", line 501, in onecmd
    return self._dispatch_cmd(handler, argv)
  File "/space/prod/osc/osc/cmdln.py", line 1232, in _dispatch_cmd
    return handler(argv[0], opts, *args)
TypeError: do_projectstaging() missing 1 required positional argument: 'cmd'

fixed

hennevogel · 2019-09-18T08:17:49Z

@lethliel whatever you need, please file issues, we want to make this work for you 🤠

lethliel · 2019-09-18T08:18:48Z

I think the API should handle this, really.

I put a work around in osc. osc now determines in which staging project the request is, when omitting the -s option

vpereira · 2019-10-02T11:40:42Z

DELETE staging/home:mstrigl/staging_projects/Staging:A/staged_requests

@lethliel

I'm not sure, but why asking the user to provide the staging workflow project, if it could be inferred from the staging project, why not use it?

i.e:

dosc ps rq add home:Admin -s home:Admin:Staging:A -r 1 IMO should be dosc ps rq add -s home:Admin:Staging:A -r 1

coolo · 2019-10-02T11:44:02Z

How to retrieve the staging workflow for a project through API?

vpereira · 2019-10-02T11:54:48Z

How to retrieve the staging workflow for a project through API?

Nothing is set in stone. The API is under development and should and will be adapted if necessary. The question should be always if the design make sense or not. Do you need an endpoint for that? Couldn't we solve it with one extra request?

dosc ps rq add -s home:Admin:Staging:A -r 1 would be probably the best way to go

coolo · 2019-10-02T12:01:34Z

The question is basically: does it make sense at all that the route is /staging/openSUSE:Leap:15.2/staging_projects/openSUSE:Leap:15.2:Staging:N/staged_requests if it could be /staging_projects/openSUSE:Leap:15.2:Staging:N/staged_requests

vpereira · 2019-10-02T12:08:56Z

The question is basically: does it make sense at all that the route is /staging/openSUSE:Leap:15.2/staging_projects/openSUSE:Leap:15.2:Staging:N/staged_requests if it could be /staging_projects/openSUSE:Leap:15.2:Staging:N/staged_requests

It is a good question.. I think we were following the rails convention and the route ended up like that.. we can file an issue for that. How should the osc command look like for you @coolo ?

coolo · 2019-10-02T12:16:28Z

osc staging -p openSUSE:Leap:15.2 select N bash is what I use

vpereira · 2019-10-02T12:18:12Z

If in the server side we had something like Project.find_by(name: 'home:Admin:Staging:A').staging_workflow then osc sp rq add -s home:Admin:Staging:A -r 1 would do the trick.

coolo · 2019-10-02T12:19:43Z

I really see no point in typing the extra :Staging: - it's a convention we will stick to

vpereira · 2019-10-02T12:34:29Z

I really see no point in typing the extra :Staging: - it's a convention we will stick to

That's a requirement from our PO. The Staging projects are just projects. No special names or hierarchy (i.e sub-projects)

coolo · 2019-10-02T12:43:41Z

Yeah, then just don't ask me. Because I can only tell you what users want not what POs want.

vpereira · 2019-10-02T13:02:59Z

Yeah, then just don't ask me. Because I can only tell you what users want not what POs want.

right @coolo but thinking about the user point of view, would it make sense to have something like dosc ps rq list -s home:Admin:Staging:A instead of dosc ps rq list home:Admin -s home:Admin:Staging:A or are you ok having to enter the project twice?

coolo · 2019-10-02T13:21:02Z

No, because in my world ps rq list home:Admin -s A is sufficient information

vpereira · 2019-10-07T06:54:53Z

@lethliel I see some difference between the example workflows given my the osc ps and how it really works:

i.e:

Example Workflow:
...
osc ps request list -s myprj:Staging:A

but what really works is:

osc ps request list myprj -s myprj:Staging:A

vpereira · 2020-01-09T12:50:00Z

As it is today, this command is broken. The XML format that we should send while staging a new request is similar with:

<requests><request id="9"/><request id="12"/></requests>

Today we are sending something like:

<requests><number>1</number></requests>

Please update it @lethliel

lethliel · 2022-09-21T08:46:40Z

This will be closed.. Nobody seems to be using it..
And rebasing on the current codebase would be more effort than to reimplement this

vpereira · 2022-09-21T11:07:07Z

This will be closed.. Nobody seems to be using it..
And rebasing on the current codebase would be more effort than to reimplement this

I think after 3 years, people that need it, just gave up of using osc and instead coded their own glue code around staging.

hennevogel · 2022-09-21T16:20:06Z

We still want this...

lethliel · 2022-09-26T19:39:59Z

This will be closed.. Nobody seems to be using it..
And rebasing on the current codebase would be more effort than to reimplement this

I think after 3 years, people that need it, just gave up of using osc and instead coded their own glue code around staging.

There is also a staging plugin in osc-relese-tools. But I guess this is for the old staging API?

lethliel · 2022-09-26T19:42:07Z

We still want this...

@hennevogel Then I will rebase and commit what I have so far.. So that the people can use it and report bugs ;-)

dmach · 2023-03-09T14:52:23Z

This pull request has been in a draft state for quite some time.
Could you either finish it, close it or add a comment that you're still planning to work on it?

dmach · 2023-03-29T14:29:00Z

Since this new command has subcommands, it might be good to rewrite it to use the new code from #1285 once it gets merged.

lethliel force-pushed the new_staging branch from 6963bda to 14ade36 Compare September 12, 2019 10:17

lethliel force-pushed the new_staging branch from 14ade36 to ecb82cc Compare September 12, 2019 12:07

lethliel force-pushed the new_staging branch from ecb82cc to a83d1e8 Compare September 17, 2019 12:10

coolo mentioned this pull request Sep 17, 2019

Delete from staging project not working openSUSE/open-build-service#8364

Closed

lethliel force-pushed the new_staging branch from a83d1e8 to 96e8c9c Compare September 18, 2019 08:16

dmach marked this pull request as draft June 2, 2022 08:17

lethliel closed this Sep 21, 2022

hennevogel reopened this Sep 21, 2022

add support for new staging api. (command osc ps) #631

Are you sure you want to change the base?

add support for new staging api. (command osc ps) #631

Conversation

lethliel commented Sep 12, 2019

lethliel commented Sep 12, 2019

vpereira commented Sep 12, 2019

marcus-h commented Sep 12, 2019 via email

marcus-h commented Sep 12, 2019 via email

lethliel commented Sep 12, 2019 via email

coolo commented Sep 17, 2019

coolo commented Sep 17, 2019

lethliel commented Sep 17, 2019

coolo commented Sep 17, 2019

coolo commented Sep 17, 2019

lethliel commented Sep 17, 2019

coolo commented Sep 17, 2019

lethliel commented Sep 18, 2019

hennevogel commented Sep 18, 2019

lethliel commented Sep 18, 2019

vpereira commented Oct 2, 2019

coolo commented Oct 2, 2019

vpereira commented Oct 2, 2019 • edited Loading

coolo commented Oct 2, 2019

vpereira commented Oct 2, 2019

coolo commented Oct 2, 2019

vpereira commented Oct 2, 2019

coolo commented Oct 2, 2019 • edited by vpereira Loading

vpereira commented Oct 2, 2019 • edited Loading

coolo commented Oct 2, 2019

vpereira commented Oct 2, 2019

coolo commented Oct 2, 2019

vpereira commented Oct 7, 2019 • edited Loading

vpereira commented Jan 9, 2020

lethliel commented Sep 21, 2022

vpereira commented Sep 21, 2022

hennevogel commented Sep 21, 2022

lethliel commented Sep 26, 2022

lethliel commented Sep 26, 2022

dmach commented Mar 9, 2023

dmach commented Mar 29, 2023

vpereira commented Oct 2, 2019 •

edited

Loading

coolo commented Oct 2, 2019 •

edited by vpereira

Loading

vpereira commented Oct 2, 2019 •

edited

Loading

vpereira commented Oct 7, 2019 •

edited

Loading