Issue #138 - Update GUI static file configuration parameters

[Futabay]

No description provided.

[MJJoyce]

Hey @Futabay, thanks for throwing this up. It looks like there's some initial work that was done in line with this at https://github.com/NASA-AMMOS/AIT-GUI/blob/master/ait/gui/__init__.py#L282

I like the approach of keeping it internal to the Plugin better than globally. Perhaps the code in the plugin could be extended (if necessary for functionality) and documentation could be updated to reflect this config option in this PR? This is definitely something users will want to change so we want to make sure the options aren't hidden.

This will also need updates pushed to Core config if there's still the "old" GUI config listed in there (note, I'm not sure there is, just a note for us to check that).

[MJJoyce]

See prev comment for change ideas

[Futabay]

Hi @MJJoyce, Thank you for the review. I will remove the line specifying User and update documentation and Core config.

[MJJoyce]

Hey @Futabay,

Thanks for throwing some changes. I apologize because I don't think I explained what I was getting at clearly in my previous post. Let me see if I can elaborate on what we want to see here vs what we have in master

We want all Plugin config to be passed via the "grouped plugin config" that users define in their server code. For the GUI we'd have something on the order of the following:

    - plugin:
        name: ait.gui.AITGUIPlugin
        inputs:
            - log_stream
            - telem_stream
        outputs:
            - command_stream

Extra plugin keys beyond the necessary name, inputs, and outputs are just passed along with the config to the Plugin during initialization and are accessible to the code. The __init__ code in master already sort of does this by extracting the User HTML directory from kwargs.

    def __init__(self, inputs, outputs, zmq_args=None, **kwargs):
        super(AITGUIPlugin, self).__init__(inputs, outputs, zmq_args, **kwargs)

        try:
            HTMLRoot.User = kwargs['html']['directory']
        except:
            pass

        bottle.TEMPLATE_PATH.append(HTMLRoot.User)

        gevent.spawn(self.init)

However, the current HTMLRoot code pulls a location out of the config. This config inspection is using the wrong location (which your PR fixed) and not really keeping config properly isolated given the new plugin approach (which your PR doesn't address).

class HTMLRoot:
    Static = pkg_resources.resource_filename('ait.gui', 'static/')
    User   = ait.config.get('gui.html.directory', Static)

It seems like the best approach is to default User to Static in HTMLRoot and leave the custom User parsing in the GUI plugin.

Thoughts?

[Futabay]

Hi @MJJoyce
Following change have been made base on the comment above and conversation we had:
-Set HTMLRoot.User to the same path of Static in class HTMLRoot:
-Add example of config.yaml static dir setting in the document.

Is it a good idea to put this try/except clause inside of class AITGUIPlugin(Plugin):? Since this is GUI plugin related?

AIT-GUI/ait/gui/__init__.py

Lines 264 to 271 in 66dfe59

	try:
	with open(os.path.join(HTMLRoot.Static, 'package.json')) as infile:
	package_data = json.loads(infile.read())
	VERSION = 'AIT GUI v{}'.format(package_data['version'])
	log.info('Running {}'.format(VERSION))
	except:
	VERSION = ''
	log.warn('Unable to determine which AIT GUI Version is running')

[MJJoyce]

Changes look good @Futabay!

What do you think about adding some logging into the try / except when we're updating the static file locations?

        try:
            HTMLRoot.User = kwargs['html']['directory']
        except:
            pass

Would be nice to notify users if the path is set or if it's not.

Regarding the other changes, seems reasonable to open a ticket to make that change as well. I'm not sure that there's anything relying on it existing outside of the plugin. We could review that on the other ticket though.

[Futabay]

ok! I will add the log, and create a separate ticket for the gui version log try/except clause

[MJJoyce]

Drop this pass

[MJJoyce]

@Futabay,

Changes look great! Thanks for the updates. There's an extra pass that got left in the try/except block that needs removed. Other than that, looks good to merge!

[Futabay]

Hi @MJJoyce Sorry the pass was left in there. Change has been pushed.

[MJJoyce]

Awesome, thanks @Futabay!