[WFCORE-6541] Patch high level command should be completely disabled … #5701
Conversation
github-actions
bot
added
the
deps-ok
This comment was marked as outdated.
This comment was marked as outdated.
| // mode to patch legacy host controllers. Therefore, we allow the Help Command to progress with "patch" command | ||
| // only in domain mode. | ||
| if ("patch".equals(mainCommand) && !ctx.isDomainMode()) { | ||
| throw new CommandException("The command is not available in the current context."); |
There was a problem hiding this comment.
This should say something about standalone/domain, as this text isn't helpful enough for 'help' :)
There was a problem hiding this comment.
@yersan , you added a comment, I am wandering if the Exception message should be evolved with more information?
There was a problem hiding this comment.
I misunderstood the first revision from @bstansberry , he was talking about the message thrown by the exception and I thought he was talking about the code comments.
The Exception message is printed directly in the CLI output, so it can also be used to give more context.
So we could use it to show more information. The difficulties are what sort of message we want to add here. I could suggest the following:
"
The 'patch' command is only available in a Domain Mode to patch legacy Host Controllers but is deactivated for standalone or disconnected sessions. See 'help installer' to get more information about how to patch your server.
"
Maybe we could review the "installer" help page to also explain when you have to use the Prospero tool and when the installer. Notice the installer command is not available on embedded Server / Host controllers, basically because we need to restart a running server to apply the update.
I see both options, allowing "help patch" to progress and adding a more descriptive text there sounds also reasonable.
There was a problem hiding this comment.
@jfdenise Could you add your opinion about this one?
I guess, I am just waiting to know whether we want to fix this by describing the information in the "patch" help page without removing the availability of "help patch" or by removing the ability of the "help patch" and showing the above message in the exception
There was a problem hiding this comment.
@yersan , I feel that I prefer the help doc to be updated. The command exists but is just enabled in some cases. The help doc could explain.
There was a problem hiding this comment.
Thanks @jfdenise , I've updated the PR accordingly
There was a problem hiding this comment.
@jfdenise Could you review again? thanks!
|
@jfdenise approved this so it's fine for now (except my wording comment), but is this how we want 'help' to work in the long run? Or should help not be limited by context like this and it's the help text itself that explains any limitations. I can see it either way, but my impression is this is the first time we've limited 'help' in this way, despite having other implementations of CommandActivator. |
That's a good point @bstansberry . If the "patch" command is executed in an invalid context, the second action to take by the user would be to inspect the help to get more information about why. The help can explain why it is not valid for the current context. |
…h command is only activated in domain mode Jira issue: https://issues.redhat.com/browse/WFCORE-6541
|
Core -> Full Integration Build 13121 outcome was UNKNOWN using a merge of 147934b |
|
Core -> Full Integration Build 12897 outcome was UNKNOWN using a merge of 147934b |
This comment was marked as off-topic.
This comment was marked as off-topic.
yersan
added
the
ready-for-merge
…in standalone mode
Jira issue: https://issues.redhat.com/browse/WFCORE-6541