jclouds-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrew Phillips <notificati...@github.com>
Subject Re: [jclouds/jclouds-cli] Fix the script engine in the jclouds CLI (#35)
Date Thu, 10 Nov 2016 00:51:29 GMT
demobox 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());

> That will always be null, but, it is an extra check that could save us some debugging
if we change things in the future (unlikely, but it's a cheap check).

@nacx Unless I'm missing something, the check would ensure that we don't overwrite any ScriptEngineManagers
that miraculously happen to be set. But I think the intention here is precisely to set an
SEM explicitly, even if it were somehow already set? That way, I think we can at least be
sure that all commands will be initialized the same way.

A comment along the lines of `setting ScriptEngineManager explicitly as it's expected to be
null` or so might help make that clearer.

And yes, I think that can easily wait until after 2.0.0 ;-)

-- 
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
Mime
View raw message