[osc1.x] staging accept does not cleanup linked packages before deleting an adi

[DimStar77]

Staging accept seems to have an issue when accepting an adi project that contained an SR with two spec files (thus staging select created a linked package for the 2nd flavor)

upon staging accept, I ran into openSUSE/osc#1247

But that error should only happen if we did try to delete the adi project without first emptying it - and not specifying to delete it recursively

the respective osc core code:

def delete_project(apiurl: str, prj: str, force=False, msg=None, recursive=False):
    if not recursive:
        packages = meta_get_packagelist(apiurl, prj)
        if packages:
            error_msg = \
                "Project contains packages. It must be empty before deleting it. " \
                "If you are sure that you want to remove this project and all its " \
                "packages use the 'recursive' option."
            raise oscerr.ProjectError(prj, error_msg)

    query = {}
    if force:
        query['force'] = "1"
    if msg:
        query['comment'] = msg
    u = makeurl(apiurl, ['source', prj], query)
    http_DELETE(u)

[DimStar77]

The old delete_project was simply:

def delete_project(apiurl, prj, force=False, msg=None):
    query = {}
    if force:
        query['force'] = "1"
    if msg:
        query['comment'] = msg
    u = makeurl(apiurl, ['source', prj], query)
    http_DELETE(u)

so our 'force attempt delete' was sufficient then

stagingapi.py does:

        try:
            delete_project(self.apiurl, project, force=True)
        except HTTPError as e:
            print(e)

Seems either osc treats force as implicit recursive or we pass recursive=true to the function (but then we have to take care of the different calls for 0.x vs 1.x)

[Vogtinator]

Seems either osc treats force as implicit recursive or we pass recursive=true to the function (but then we have to take care of the different calls for 0.x vs 1.x)

Yep, or do the API call ourselves

[dirkmueller]

Can we simply fix osc?

[Vogtinator]

It looks like an intentional change. Did you think of something like if not recursive and not force:?

[dmach]

Seems either osc treats force as implicit recursive or we pass recursive=true to the function (but then we have to take care of the different calls for 0.x vs 1.x)

How about the following?

sig = inspect.signature(osc.core.delete_project)
delete_project_kwargs = {}
if "recursive" in sig:
    delete_project_kwargs["recursive"] = True
osc.core.delete_project(..., **delete_project_kwargs)

Another option would be changing the default in osc from recursive=False to recursive=True, but I'd prefer to have safe defaults.

[DimStar77]

tested a checkin round with this patch:

diff --git a/osclib/stagingapi.py b/osclib/stagingapi.py
index 62abb9df..a9b8d36d 100644
--- a/osclib/stagingapi.py
+++ b/osclib/stagingapi.py
@@ -5,12 +5,12 @@ import dateutil.parser
 import logging
 import textwrap
 from urllib.error import HTTPError, URLError
+from inspect import signature
 
 import time
 import re
 from lxml import etree as ET
 
-
 from osc import conf
 from osc import oscerr
 from osclib.core import attribute_value_load
@@ -1533,6 +1533,10 @@ class StagingAPI(object):
             return
 
         try:
-            delete_project(self.apiurl, project, force=True)
+            sig = signature(delete_project)
+            if "recursive=" in str(sig):
+                delete_project(self.apiurl, project, force=True, recursive=True)
+            else:
+                delete_project(self.apiurl, project, force=True)
         except HTTPError as e:
             print(e)

[Vogtinator]

tested a checkin round with this patch:

Successfully? If so -> PR!