Juan Hernandez has posted comments on this change.
Change subject: packaging: [DO NOT MERGE] Handling active tasks during upgrade
......................................................................
Patch Set 1: (8 inline comments)
....................................................
File packaging/fedora/setup/engine-upgrade.py
Line 17:
Line 18: # Consts
Line 19:
Line 20: WAIT_PERIOD = 60
Line 21: MAX_CYCLES = 20
Can we make explicit the fact that we use this for maintenance mode? Something
like MAINTENANCE_TASKS_WAIT_PERIOD and MAINTENANCE_TASKS_CYCLES, or/and a
comment explaining it.
Line 22:
Line 23: #TODO: Work with a real list here
Line 24: RPM_LIST = """
Line 25: ovirt-engine
Line 63: #MSGS
Line 64: MSG_STOP_RUNNING_TASKS = "Info: There are following running tasks and
compensations \
Line 65: found in the manager:\n\nTasks:\n%s\n\nCompensations:\n%s\n\n\
Line 66: Would you like to proceed with the upgrade (and try to stop tasks
automatically)?\n Answering \
Line 67: 'no' will stop the upgrade."
Not for this patch, but we should start to use the "textwrap" python module to
avoid manually wrapping text.
Line 68: MSG_RUNNING_TASKS_CLEARED = "Info: no running tasks found. Continue"
Line 69: MSG_ERROR_USER_NOT_ROOT = "Error: insufficient permissions for user
%s, you must run with user root."
Line 70: MSG_NO_ROLLBACK = "Error: Current installation "
Line 71: MSG_RC_ERROR = "Return Code is not zero"
Line 747: True,
Line 748: MSG_ERROR_CONNECT_DB
Line 749: )
Line 750:
Line 751: def getRunningTasks():
Would it make sense to have to separate methods? One to get the running tasks
and another one to get compensations. That way you don't have to return a
dictionary, but a list, and that way you don't need the "async_tasks" and
"compensation" keys.
Line 752:
Line 753: runningTasks = {}
Line 754:
Line 755: # Get async tasks:
Line 781: return runningTasks
Line 782:
Line 783: def checkRunningTasks():
Line 784: # Find running tasks first
Line 785: runningTasks = getRunningTasks()
compensations = getCompensations()
Line 786:
Line 787: if len(runningTasks['async_tasks']) > 0 or \
Line 788: len(runningTasks['compensations']) > 0:
Line 789:
Line 784: # Find running tasks first
Line 785: runningTasks = getRunningTasks()
Line 786:
Line 787: if len(runningTasks['async_tasks']) > 0 or \
Line 788: len(runningTasks['compensations']) > 0:
if runningTasks or compensations:
Line 789:
Line 790: try:
Line 791: # TODO: update runningTasks names/presentation and
compensations
Line 792: answerYes = utils.askYesNo(MSG_STOP_RUNNING_TASKS %
Line 804: # Pull tasks in a loop for some time
Line 805: # WAIT_PERIOD = 60 (seconds, between trials)
Line 806: # MAX_CYCLES = 20 (how many times to try)
Line 807: count = 0
Line 808: while runningTasks > 0 and count < MAX_CYCLES:
The expression "runningTasks > 0" always evaluates to True. I guess you meant
just "runningTasks" or "len(runningTasks) > 0".
Line 809: time.sleep(WAIT_PERIOD)
Line 810: runningTasks = getRunningTasks()
Line 811: count += 1
Line 812:
Line 810: runningTasks = getRunningTasks()
Line 811: count += 1
Line 812:
Line 813: # Restore normal configuration
Line 814: restoreNormalMode()
This action should probably go in a "finally" block, so that we restore the
mode even if there are exceptions while we wait for the tasks and compensations
to be cleaned.
Line 815:
Line 816: # Check if there are still running tasks
Line 817: if len(runningTasks) > 0:
Line 818: # There are still tasks running, so ask user what to
do
Line 816: # Check if there are still running tasks
Line 817: if len(runningTasks) > 0:
Line 818: # There are still tasks running, so ask user what to
do
Line 819: raise Exception("There are still running tasks.
Please make sure \
Line 820: that there are no running before you
continue. \
I miss something in this message ".. there are no running *what* before ...".
Line 821: Stopping upgrade.")
Line 822:
Line 823:
Line 824: except:
--
To view, visit http://gerrit.ovirt.org/8740
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I90abb3d1e6ad0ac5557485e40064e811fc87f491
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alex Lourie <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Barak Azulay <[email protected]>
Gerrit-Reviewer: Eli Mesika <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Kiril Nesenko <[email protected]>
Gerrit-Reviewer: Moran Goldboim <[email protected]>
Gerrit-Reviewer: Ofer Schreiber <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
