Skip to content

/ jbosstools-server

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JBIDE-18372 - tweaking progress monitors #307

Merged
merged 1 commit into from Dec 4, 2014
+32 −7
Merged

JBIDE-18372 - tweaking progress monitors #307

Changes from all commits
Commits
Show all changes
1 commit
Select commit
dba94d3
JBIDE-18372 - tweaking progress monitors
robstryker Nov 17, 2014
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

27 ...ss/tools/as/core/server/controllable/subsystems/internal/ManagementPublishController.java
View file
Open in desktop
@@ -240,15 +240,17 @@ public int publishModule(int kind, int deltaKind, IModule[] module,
return removeModule(module, monitor);
}


monitor.beginTask("Publishing " + module[0].getName(), 1000); //$NON-NLS-1$

// ServerBehaviourDelegate already started this monitor with a 1000 total work to do
monitor.setTaskName("Publishing " + module[0].getName()); //$NON-NLS-1$

boolean isBinaryObject = ServerModelUtilities.isBinaryModule(module);
File[] toTransfer = null;
if( !isBinaryObject ) {
File file = zipLocally(module[0], publishType, ProgressMonitorUtil.submon(monitor, 100));
monitor.setTaskName("Zipping module: " + module[0].getName()); //$NON-NLS-1$
IProgressMonitor submon = ProgressMonitorUtil.submon(monitor, 200);
File file = zipLocally(module[0], publishType, submon);
toTransfer = new File[]{file};
submon.done();
} else {
IModuleResource[] resources = ModuleResourceUtil.getMembers(module[0]);
// How to handle binary modules??
@@ -262,29 +264,36 @@ public int publishModule(int kind, int deltaKind, IModule[] module,
}
}
toTransfer = (File[]) fileList.toArray(new File[fileList.size()]);
monitor.worked(200);
}

if( toTransfer != null ) {
MultiStatus ms = new MultiStatus(JBossServerCorePlugin.PLUGIN_ID, IEventCodes.JST_PUB_FAIL, "Deployment of module " + module[0].getName() + " has failed", null); //$NON-NLS-1$//$NON-NLS-2$
IProgressMonitor transferMonitor = ProgressMonitorUtil.submon(monitor, 900);
IProgressMonitor transferMonitor = ProgressMonitorUtil.submon(monitor, 800);
transferMonitor.beginTask("Transfering " + module[0].getName(), 100*toTransfer.length); //$NON-NLS-1$
for( int i = 0; i < toTransfer.length; i++ ) {

// Maybe name will be customized in UI?
JBoss7DeploymentState state = getService().getDeploymentState(managementDetails, toTransfer[i].getName());
if( state != JBoss7DeploymentState.NOT_FOUND) {
IJBoss7DeploymentResult removeResult = getService().undeploySync(managementDetails, toTransfer[i].getName(), true, ProgressMonitorUtil.submon(transferMonitor, 10));
monitor.setTaskName("Undeploying: " + toTransfer[i].getName()); //$NON-NLS-1$
IJBoss7DeploymentResult removeResult = getService().undeploySync(managementDetails, toTransfer[i].getName(), true, ProgressMonitorUtil.submon(transferMonitor, 5));
ms.add(removeResult.getStatus());
} else {
transferMonitor.worked(5);
}

monitor.setTaskName("Transfering: " + toTransfer[i].getName()); //$NON-NLS-1$
IJBoss7DeploymentResult result = getService().deploySync(managementDetails, toTransfer[i].getName(), toTransfer[i],
true, ProgressMonitorUtil.submon(transferMonitor, 100));
true, ProgressMonitorUtil.submon(transferMonitor, 95));
IStatus s = result.getStatus();
ms.add(s);
}
if( ms.isOK()) {
return IServer.PUBLISH_STATE_NONE;
}
ServerLogger.getDefault().log(getServer(), ms);
transferMonitor.done();
return IServer.PUBLISH_STATE_FULL;
} else {
// TODO error?
@@ -308,10 +317,13 @@ private String getDeploymentOutputName(IServer server, IModule module) throws Co
public void publishServer(int kind, IProgressMonitor monitor)
throws CoreException {
// intentionally blank
monitor.beginTask("", 100); //$NON-NLS-1$

This comment has been minimized.

Sign in to view
Copy link
@maxandersen

maxandersen Dec 4, 2014

Member

what is the point of this begin and then mark as done immediately ?

This comment has been minimized.

Sign in to view
Copy link
@robstryker

robstryker Dec 4, 2014

Author Member

So I tested it a lot with and without these parts. Theoretically, I should have similar lines for publishStart and publishFinish. WTP is allocating sub monitors for these methods, and never marks them as done. It's my job to start, and then done() them.

The problem is if I have the monitors in publishStart and publishFinish and publishServer, then the actual publish (zip plus transfer) is really only like 40% of the monitor (depending on the number of modules being published, but for 1 module being published, it's 40%). Basically, their weights are kinda... weird.

So I decided to keep the start() and done() in only publishServer, not publishStart / publishFinish (for THIS publish method). It makes it so the progress monitor advances to about 15%, and when the publish is basically complete, the monitor is at around 78%.

If I were to put it into all three of them, then the progress monitor would read like 40% before I even did anything, and around 80% afterwards.

At this point it's kind of a value judgement and what I feel works best for the user... to see some progress in the beginning, then the slow increase during zip and mgmt calls, and then a fast end.

Tested with an example project with 5000 files in it.
monitor.done();
}


private File zipLocally(IModule module, int publishType, IProgressMonitor monitor) throws CoreException {
monitor.beginTask("Zip module " + module.getName(), 100); //$NON-NLS-1$
// Zip into a temporary folder, then transfer to the proper location
IPath localTempLocation = getMetadataTemporaryLocation(getServer());
String name = getDeploymentOutputName(getServer(), module);
@@ -325,6 +337,7 @@ private File zipLocally(IModule module, int publishType, IProgressMonitor monito
} else if( publishType == PublishControllerUtility.INCREMENTAL_PUBLISH) {
result = runner.incrementalPublishModule(ProgressMonitorUtil.submon(monitor, 100));
}
monitor.done();
if( result != null && result.isOK()) {
if( tmpArchive.toFile().exists()) {
return tmpArchive.toFile();
6 ...as.management.as71/src/org/jboss/ide/eclipse/as/internal/management/as71/AS71Manager.java
View file
Open in desktop
@@ -118,15 +118,21 @@ public void handle(Callback[] callbacks) throws IOException,

public IJBoss7DeploymentResult undeploySync(String name, boolean removeFile, IProgressMonitor monitor)
throws JBoss7ManangerException {
monitor.beginTask("Undeploy via management: " + name, 100);
IJBoss7DeploymentResult result = undeploy(name, removeFile);
result.getStatus();
monitor.worked(100);
monitor.done();
return result;
}

public IJBoss7DeploymentResult deploySync(String name, File file, boolean add, IProgressMonitor monitor)
throws JBoss7ManangerException {
monitor.beginTask("Deploy via management: " + name, 100);
IJBoss7DeploymentResult result = deploy(name, file, add);
result.getStatus();
monitor.worked(100);
monitor.done();
return result;
}

6 ...t.wildfly8/src/org/jboss/ide/eclipse/as/internal/management/wildfly8/Wildfly8Manager.java
View file
Open in desktop
@@ -115,15 +115,21 @@ public void handle(Callback[] callbacks) throws IOException,

public IJBoss7DeploymentResult undeploySync(String name, boolean removeFile, IProgressMonitor monitor)
throws JBoss7ManangerException {
monitor.beginTask("Undeploy via management: " + name, 100);
IJBoss7DeploymentResult result = undeploy(name, removeFile);
result.getStatus();
monitor.worked(100);
monitor.done();
return result;
}

public IJBoss7DeploymentResult deploySync(String name, File file, boolean add, IProgressMonitor monitor)
throws JBoss7ManangerException {
monitor.beginTask("Deploy via management: " + name, 100);
IJBoss7DeploymentResult result = deploy(name, file, add);
result.getStatus();
monitor.worked(100);
monitor.done();
return result;
}

ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.