nacx commented on this pull request.
> @@ -344,7 +349,17 @@ private void discoverCommands(CommandProcessorImpl
> commandProcessor, ClassLoader
@Override
public Action createNewAction() {
try {
- return ((Class<? extends Action>)
actionClass).newInstance();
+ Action action = ((Class< ? extends Action>)
actionClass).newInstance();
+ if (action instanceof ComputeCommandBase) {
+ ShellTableFactory shellTableFactory =
((ComputeCommandBase) action).getShellTableFactory();
+ if (shellTableFactory instanceof
BasicShellTableFactory) {
+ BasicShellTableFactory factory =
(BasicShellTableFactory) shellTableFactory;
+ if (factory.getScriptEngineManager() ==
null) {
+ factory.setScriptEngineManager(new
ScriptEngineManager());
Well, we could do that, but *coupling* jclouds-karaf to this.
The engine is now automatically injected in OSGi by the framework itself. We
define the commands and the properties and they are injected. Previous versions
of jclouds-karaf initialized the engine manager to the value we're setting
here, but that implementation happened to not be compatible with OSGi. That
motivated the change to an OSGi compatible implementation and an injection
based approach.
The CLI, however, is manually instantiating the commands *outside* an OSGi
context. IT manually creates the classes after reading their names from the
META-INF file, and thus, injection does not happen. Also we can't use the OSGi
version, since we need the bundle context to initialise it, and we don't have
it here (at least I don't know how to get it), that's why I'm setting the
default engine manager, which is the instance that happened to work in previous
versions. In fact, in previous versions the interactive mode failed due to an
invalid engine in karaf, and the shell script worked. Now each part has the
right engine set.
Regarding the way to set the instance, we should ideally find a way to make the
OSGi injection happen here, but I don't know if that is possible in the CLI
runner context.
Another option would be to create a constructor in each command that accepts a
ScriptEngineManager as a parameter so we can pass the instance also when
instantiating the command manually from the CLI, but I thought that would be
coupling the commands in jclouds-karaf to a very particular use case where
someone manually instantiates the commands outside an OSGi environment. I
preferred to leave jclouds-karaf clean, and add this ugly workaround in the CLI.
Thoughts?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-cli/pull/35
