Alon Bar-Lev has posted comments on this change.
Change subject: frontend: Non-plugin automatic invocation of console session
......................................................................
Patch Set 4: (4 inline comments)
....................................................
File
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/uicommon/SpiceNativeImpl.java
Line 21:
Line 22:
configBuilder.append(lineSeparator).append("type=spice")//$NON-NLS-1$
Line 23:
.append(lineSeparator).append("host=").append(getHost())//$NON-NLS-1$
Line 24:
.append(lineSeparator).append("password=").append(getPassword())//$NON-NLS-1$
Line 25:
.append(lineSeparator).append("port=").append(Integer.toString(getPort()));//$NON-NLS-1$
Don't you have String.format in gwt?
config=String.format(
(
"[virt-viewer]\n" +
"host=%1$s\n" +
"password=%2$s\n" +
"..."
),
getHost(),
getPassword(),
...
)
Line 26:
Line 27:
ConsoleModel.makeConsoleConfigRequest(configBuilder.toString());
Line 28: }
Line 29:
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/ConsoleModel.java
Line 272: }
Line 273:
Line 274: public static void makeConsoleConfigRequest(String
configFileContent) {
Line 275: FormPanel formPanel = new FormPanel(new NamedFrame("_self"));
//$NON-NLS-1$
Line 276: formPanel.setMethod(FormPanel.METHOD_POST);
Nice and easy! I thought it will be more complex.
Line 277:
Line 278: VerticalPanel innerPanel = new VerticalPanel();
Line 279: innerPanel.add(buildTextArea("filename",
"console.vv"));//$NON-NLS-1$ $NON-NLS-2$
Line 280: innerPanel.add(buildTextArea("contenttype",
"x-virt-viewer"));//$NON-NLS-1$ $NON-NLS-2$
Line 277:
Line 278: VerticalPanel innerPanel = new VerticalPanel();
Line 279: innerPanel.add(buildTextArea("filename",
"console.vv"));//$NON-NLS-1$ $NON-NLS-2$
Line 280: innerPanel.add(buildTextArea("contenttype",
"x-virt-viewer"));//$NON-NLS-1$ $NON-NLS-2$
Line 281: innerPanel.add(buildTextArea("content",
configFileContent));//$NON-NLS-1$
This is detached logic... the configuration content and the content type are
derived from each other, there is no sense in setting one in one place and the
other in other place. The filename and contenttype should be parameters to this
function as well.
Line 282:
Line 283: formPanel.setWidget(innerPanel);
Line 284: formPanel.setAction(CONSOLE_CONFIG_SERVLET_URL);
Line 285:
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VncConsoleModel.java
Line 93:
.append(lineSeparator).append("host=").append(hostName)//$NON-NLS-1$
Line 94:
.append(lineSeparator).append("password=").append(otp64)//$NON-NLS-1$
Line 95:
.append(lineSeparator).append("port=").append(getEntity().getDisplay().toString());//$NON-NLS-1$
Line 96:
Line 97:
ConsoleModel.makeConsoleConfigRequest(configBuilder.toString());
Should also pass the content type and attachment name here, as they are related
to each other. If we have different implementation then we should not change
the makeConsoleConfigRequest()
Line 98: }
Line 99:
Line 100: @Override
Line 101: protected void UpdateActionAvailability()
--
To view, visit http://gerrit.ovirt.org/11703
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib31e870deb7ecb3dac4cff25e49a3ebca4706a25
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Frank Kobzik <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Frank Kobzik <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Tomas Jelinek <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
