Skip to content

Commit 02c404b

Browse files
author
Nicolas Georges
committed
Fix import new appversion with existing parent app
import_file/sync_index/sync_dir commands would previously overwrite the top-level application record when importing a new app version. This patch fixes that: a top-level application record can now only be created, once, when it doesn't exist, and never updated afterwards.
1 parent 0b2e606 commit 02c404b

1 file changed

Lines changed: 63 additions & 30 deletions

File tree

vipapps/vipapps.py

Lines changed: 63 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,14 @@ def init_api() -> None:
4444
vip.setApiKey(vip_apikey)
4545
init_api_done = True
4646

47-
# make app object from GET /rest/admin/appVersions response
48-
def convert_app_version(av):
47+
# make parentapp object from GET /rest/admin/applications response
48+
def convert_parentapp(app):
49+
name = app["name"]
50+
result = {"name":name,"owner":app["owner"],"citation":app["citation"],"groups":app["applicationGroups"],"public":app["public"]}
51+
return result
52+
53+
# make appversion object from GET /rest/admin/appVersions response
54+
def convert_appversion(av):
4955
name = av["applicationName"]
5056
identifier = name+"/"+av["version"]
5157
desc = json.loads(av["descriptor"])
@@ -54,14 +60,33 @@ def convert_app_version(av):
5460
result["source"] = av["source"]
5561
return result
5662

57-
# get_apps(): get a list of apps and descriptors from a VIP-portal instance
58-
def get_apps() -> list:
63+
# get_parentapps(): get a dict of top-level application records
64+
# from a VIP-portal instance
65+
def get_parentapps() -> list:
66+
init_api()
67+
apps = vip.generic_get("admin/applications")
68+
appnames = map(lambda app:app["name"], apps)
69+
return dict(zip(appnames, list(map(convert_parentapp, apps))))
70+
71+
# get_parentapp(): get a single top-level app
72+
def get_parentapp(appname) -> object:
73+
init_api()
74+
try:
75+
appver = vip.generic_get("admin/applications/"+urllib.parse.quote(appname))
76+
except RuntimeError as e:
77+
# XXX see get_appversion
78+
return None
79+
return convert_parentapp(appver)
80+
81+
# get_appversions(): get a list of appversions and descriptors
82+
# from a VIP-portal instance
83+
def get_appversions() -> list:
5984
init_api()
6085
app_versions = vip.generic_get("admin/appVersions")
61-
return list(map(convert_app_version, app_versions))
86+
return list(map(convert_appversion, app_versions))
6287

63-
# get_app(): get a single app
64-
def get_app(identifier) -> object:
88+
# get_appversion(): get a single appversion
89+
def get_appversion(identifier) -> object:
6590
init_api()
6691
try:
6792
appver = vip.generic_get("admin/appVersions/"+urllib.parse.quote(identifier))
@@ -71,7 +96,7 @@ def get_app(identifier) -> object:
7196
# with 200 OK would be more reliable.
7297
# print(e) # Error 8000 from VIP
7398
return None
74-
return convert_app_version(appver)
99+
return convert_appversion(appver)
75100

76101
# normalized descriptor filename, assuming name & version are already ok
77102
def descriptor_filename(appname, appversion):
@@ -290,6 +315,7 @@ class AppFields:
290315
# This should be kept explicit in command-line args.
291316

292317
# default values for a new app:
318+
parent = None
293319
owner = None
294320
groups = []
295321
citation = ""
@@ -302,7 +328,8 @@ class AppFields:
302328
source = ""
303329
# app is defined on update only, and contains the existing appversion
304330
# args is defined everytime, and contains user-provided values
305-
def __init__(self, app=None, args=None):
331+
def __init__(self, parent=None, app=None, args=None):
332+
self.parent = parent
306333
# on update, default to keeping existing values
307334
if app != None:
308335
self.is_visible = app["is_visible"]
@@ -343,27 +370,25 @@ def import_file(file, fields, is_overwrite=False, dry_run=True, verbose=False):
343370
appname = file["descriptor"]["name"]
344371
version = file["descriptor"]["tool-version"]
345372
descriptor = file["rawtext"]
346-
app = {"name":appname,"applicationGroups":fields.groups,"owner":fields.owner,"citation":fields.citation,"public":fields.public}
347-
app_url = "admin/applications/" + urllib.parse.quote(appname)
348-
appver = {"applicationName":appname,"version":version,"descriptor":descriptor,"doi":fields.doi,"visible":fields.is_visible,"resources":fields.resources,"tags":fields.tags,"settings":fields.settings,"source":fields.source}
349-
appver_url = "admin/appVersions/" + urllib.parse.quote(appname) + "/" + urllib.parse.quote(version)
350373
msg = ""
351374
if is_overwrite:
352375
msg += " (overwrite)"
353376
if dry_run:
354377
msg += " (dry run)"
355378
# never change app on update: doing so would be arguable if there are
356-
# several appversions per app, and also it avoids having to fetch app
357-
# fields on GET (see AppFields())
358-
can_put_app = not is_overwrite
379+
# several appversions per app
359380
print("importing app %s %s%s" % (appname, version, msg))
360-
if can_put_app:
381+
if fields.parent == None and is_overwrite == False:
382+
app = {"name":appname,"applicationGroups":fields.groups,"owner":fields.owner,"citation":fields.citation,"public":fields.public}
383+
app_url = "admin/applications/" + urllib.parse.quote(appname)
361384
if verbose:
362385
print("PUT %s %s" % (app_url, app))
363386
if not dry_run:
364387
r = vip.generic_put(app_url, app)
365388
if verbose:
366389
print("app updated:", r)
390+
appver = {"applicationName":appname,"version":version,"descriptor":descriptor,"doi":fields.doi,"visible":fields.is_visible,"resources":fields.resources,"tags":fields.tags,"settings":fields.settings,"source":fields.source}
391+
appver_url = "admin/appVersions/" + urllib.parse.quote(appname) + "/" + urllib.parse.quote(version)
367392
if verbose:
368393
print("PUT %s %s" % (appver_url, appver))
369394
if not dry_run:
@@ -439,12 +464,15 @@ def import_existing_app(app, file, fields, is_overwrite=False,
439464
dry_run=dry_run, verbose=verbose)
440465

441466
def import_new_app(file, fields, dry_run=True, verbose=False):
442-
print("%s: new app" % file["identifier"])
443-
import_file(file, fields, is_overwrite=False,
444-
dry_run=dry_run, verbose=verbose)
467+
if fields.parent != None:
468+
print("%s: new appversion" % file["identifier"])
469+
else:
470+
print("%s: new app+appversion" % file["identifier"])
471+
import_file(file, fields, is_overwrite=False,
472+
dry_run=dry_run, verbose=verbose)
445473

446474
# sync a list of descriptors with a list of apps (from a VIP instance)
447-
def perform_sync(args, apps, files):
475+
def perform_sync(args, parents, apps, files):
448476
# sort both lists, then do one linear pass on them
449477
# we could also use dicts and the sets of their keys.
450478
apps.sort(key=lambda app: app["identifier"])
@@ -467,7 +495,9 @@ def perform_sync(args, apps, files):
467495
app = None
468496
if file != None:
469497
# set field values from global args + per-file overrides
470-
fields = AppFields(app=app, args=args)
498+
appname = file["descriptor"]["name"]
499+
parent = parents[appname] if appname in parents else None
500+
fields = AppFields(app=app, args=args, parent=parent)
471501
if "resources" in file:
472502
fields.resources = file["resources"]
473503
if "settings" in file:
@@ -508,7 +538,7 @@ def print_app(label: str, identifier: str, desc: dict,
508538

509539
# list apps and descriptors on a VIP instance
510540
def cmd_list_apps(args):
511-
apps = get_apps()
541+
apps = get_appversions()
512542
print("found %d apps on %s:" % (len(apps), get_vip_url()))
513543
for app in apps:
514544
print_app(app["name"], app["identifier"], app["descriptor"],
@@ -547,8 +577,9 @@ def cmd_import_file(args):
547577
# check if app exists
548578
appname = file["descriptor"]["name"]
549579
version = file["descriptor"]["tool-version"]
550-
app = get_app(file["identifier"])
551-
fields = AppFields(app=app, args=args)
580+
parent = get_parentapp(appname)
581+
app = get_appversion(file["identifier"])
582+
fields = AppFields(app=app, args=args, parent=parent)
552583
if app != None: # app already exists
553584
import_existing_app(app, file, fields,
554585
is_overwrite=args.overwrite,
@@ -570,25 +601,27 @@ def cmd_check_file(args):
570601
# sync apps from a directory of boutiques descriptors to a VIP instance
571602
def cmd_sync_dir(args):
572603
# get apps list from VIP
573-
apps = get_apps()
604+
parents = get_parentapps()
605+
apps = get_appversions()
574606
# get files list, converting from dict
575607
files = get_files_from_dir(args.dirname, silent=args.silent)
576608
files = list(files.values())
577609
# sync
578-
perform_sync(args, apps, files)
610+
perform_sync(args, parents, apps, files)
579611

580612
# sync apps from a CSV index file to a VIP instance
581613
def cmd_sync_index(args):
582614
# get apps list from VIP
583-
apps = get_apps()
615+
parents = get_parentapps()
616+
apps = get_appversions()
584617
# get files list
585618
files = get_files_from_index(args.filename, silent=args.silent)
586619
files = list(files.values())
587620
# sync
588-
perform_sync(args, apps, files)
621+
perform_sync(args, parents, apps, files)
589622

590623
def cmd_show_apps(args):
591-
print(get_apps())
624+
print(get_appversions())
592625

593626
def cmd_show_files(args):
594627
print(get_files_from_dir(args.dirname, silent=args.silent))

0 commit comments

Comments
 (0)