Ofer Schreiber has posted comments on this change. Change subject: packaging: Updated interaction with user on async tasks ......................................................................
Patch Set 3: I would prefer that you didn't submit this (3 inline comments) .................................................... Commit Message Line 10: the user, where tasks were found and user was asked whether to continue Line 11: (and if user continues, then restart engine in maintenance mode) Line 12: Line 13: If user chooses not to continue, 'finally' will kick in, and try to Line 14: restore the engine configuration, although it didn't change yet. Moving the 'try' is not the right thing to do. The "restore from maintenance" function should know when it should run and when it shouldn't Line 15: Line 16: * Also, changed the return value in getRunningTasks and getCompensation Line 17: functions from "return None" to empty string "return ''". This improves Line 18: the printouts to the user, and instead of printing "None" in tasks, .................................................... File packaging/fedora/setup/engine-upgrade.py Line 844: timestamp = "\n[ " + utils.getCurrentDateTimeHuman() + " ] " Line 845: print timestamp + output_messages.INFO_STOPPING_TASKS % MAINTENANCE_TASKS_WAIT_PERIOD_MINUTES Line 846: Line 847: # restart jboss/engine in maintenace mode (i.e different port): Line 848: utils.configureEngineForMaintenance() Actually, the try/finally should be right here. (and it should know which actions have been done before) other wise, every issue before this issue (for example, if utils.getCurrentDateTimeHuman() will throw exception) the program will try to restore from maintenance Line 849: origTimeout = utils.configureTasksTimeout(timeout=0, Line 850: engineConfigBin=engineConfigBinary, Line 851: engineConfigExtended=engineConfigExtended) Line 852: startEngine(service) Line 845: print timestamp + output_messages.INFO_STOPPING_TASKS % MAINTENANCE_TASKS_WAIT_PERIOD_MINUTES Line 846: Line 847: # restart jboss/engine in maintenace mode (i.e different port): Line 848: utils.configureEngineForMaintenance() Line 849: origTimeout = utils.configureTasksTimeout(timeout=0, The "real" issue is right here -> you're getting origTimeout too late, and when trying to restore it, it's not defined. Line 850: engineConfigBin=engineConfigBinary, Line 851: engineConfigExtended=engineConfigExtended) Line 852: startEngine(service) Line 853: -- To view, visit http://gerrit.ovirt.org/8952 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia1f1aaf7ac812d69dc81f3fb8dee5a626bc7634a Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alex Lourie <[email protected]> Gerrit-Reviewer: Alex Lourie <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[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]> Gerrit-Reviewer: Ohad Basan <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
