Skip to content
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

[WFCORE-1015] init jboss cli jconsole plugin with the remote mbean co… #1123

Closed
wants to merge 1 commit into from

Conversation

bmaxwell
Copy link
Contributor

…nnection that is established, so it can load

jira: https://issues.jboss.org/browse/WFCORE-1015

…nnection that is established, so it can load
@@ -119,6 +119,7 @@ private void connect() throws Exception {
if (mbeanServerConn instanceof RemotingMBeanServerConnection) {
final CommandContext cmdCtx = CommandContextFactory.getInstance().newCommandContext();
isConnected = connectUsingRemoting(cmdCtx, (RemotingMBeanServerConnection)mbeanServerConn);
init(cmdCtx);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't seem right to ignore the result of L121 here. The init(cmdCtx) call will end up setting isConnected = true, but if L121 returns 'false' the plugin isn't actually connected.

I haven't looked carefully, but it seems like it should be more like this:

if (connectUsingRemoting(cmdCtx, (RemotingMBeanServerConnection)mbeanServerConn)) {
    init(cmdCtx);
} else {
   doSomethingButBrianIsntSureWHAT();
}

The doSomethingButBrianIsntSureWHAT is probably display some sort of error dialog. This would mean the user has a RemotingMBeanServerConnection to something but a management channel can't be opened on it. This seems highly unlikely and the right thing to do to recover is unclear, so just failing seems reasonable.

@bstansberry bstansberry added the fixme This PR introduces issues that must be fixed label Sep 29, 2015
@bstansberry
Copy link
Contributor

Closing due to inactivity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixme This PR introduces issues that must be fixed
Projects
None yet
2 participants