separation of plugins to install

[beatrizsanchez]

Hi, I modified the UI to make the different kinds of required plugins clearer. The output is the following:

Let me know if you have any comments.

[kolovos]

Would it make sense to try to simplify the UI a bit more? Some ideas below:

• Add a "label" to the extension point that feeds the "Instance type" menu so that the user sees something like "Local" instead of "o.h.c.r.LocalHawkFactory"
• Add a flag to the same extension point to say whether the factory is local or remote and show/hide the "Local storage folder"/"Remote location" fields accordingly
• Hide "Model plugins", "Meta-Model plugins", and "Updater plugins" to an "Advanced" tab as most users are unlikely to care
• Also add a "label" to the back-end extension point so that the combo box can show "Neo4J" instead of "o.h.n.Neo4JDatabase" (probably a good opportunity to get rid of _v2 from the Neo4J plugin too ;) )

Not related to the UI, but shouldn't we rename packages to start with org.eclipse.hawk?

[beatrizsanchez]

Hi Dimitris, Thanks for the input, this makes sense. I will work on it.

[agarciadom]

Thanks for your comments, Dimitris! BTW, there are also graph change listener plugins which should be listed as well. There is one for Modelio, for instance.

I agree with the package renaming, but I wouldn't do it in this PR just yet :-). I'll do that outside this PR while we wait for any other comments from the CQ.

[beatrizsanchez]

Here is what I've done:

• Add a "label" to the extension point that feeds the "Instance type" menu so that the user sees something like "Local" instead of "o.h.c.r.LocalHawkFactory"

Done for most extension points (metamodel/model factories, model updaters, graph change listeners, query languages, vcs managers, dbs). When unavailable, I added the getHumanReadableName method in addition to the getType method which provides the fully qualified class name.

• Add a flag to the same extension point to say whether the factory is local or remote and show/hide the "Local storage folder"/"Remote location" fields accordingly

Done and this makes the dialog that creates a new index change accordingly

• Hide "Model plugins", "Meta-Model plugins", and "Updater plugins" to an "Advanced" tab as most users are unlikely to care

It is not in an advance tab in the creation dialog but in the Eclipse preferences. Surely it would be good to add an advance button in the creation dialog and in the Hawk View.

The following image shows this view where depending on the selected hawk instance the enabled/disabled plugins change.

I've also rearranged all the Hawk preferences under a Hawk category.

The following view is trying to follow the suggestion in Issue #14

• Also add a "label" to the back-end extension point so that the combo box can show "Neo4J" instead of "o.h.n.Neo4JDatabase" (probably a good opportunity to get rid of _v2 from the Neo4J plugin too ;) )

They have been renamed to a human readable version. I'll leave the removal of v2 to Antonio.

[agarciadom]

Hi Beatriz,

Thanks for your work! Using human-readable names sounds great, and it appears that we can finally change the existing plugins on Hawk instances (which has been a long overdue feature).

We're almost there, but I think we need a little bit of polish:

• api.dt now depends on ui2. Is there a reason for this? I've tried removing the dependency and everything seems fine.
• We seem to have one regression - it is not possible to change the plugins while creating a new Hawk instance, as it was possible before. Could we add back that option, perhaps as an "advanced" tab in the instance creation dialog as Dimitris suggested?
• What happens if we have (meta)models indexed through some plugins, and those plugins get disabled? Do we have a defined behaviour for it? I'd expect that the models would probably be deleted on the next sync, but we need to test and document it (and potentially give a warning to the user).
• Does it make sense to allow the updater plugin to be changed at all? The graph structure would be completely different.
• It doesn't make sense to enable/disable query engine plugins for an instance. You simply use a query engine plugin to run a query on a graph. Those should be removed from the preferences page and any future "Advanced" tab in the creation dialog.
• We are not using those human readable names for the query engines in the Query dialog.
• The capitalization style in some of the plugins is different (e.g. "Modelio metamodel resource factory"). We should standardize to "Modelio Metamodel Resource Factory", and perhaps have a second look at the others.
• By default, all plugins are disabled. This means that upon first use, the Hawk instance would not be able to index anything. I think the defaults should be flipped - we could probably enable all metamodel and model plugins, and leave all change listener plugins disabled by default.
• If I press "Start" on an instance in the "Instance Manager", the "Start"/"Stop" buttons are not updated. Same goes when I stop a running instance.
• The buttons in the "Instance Manager" have different sizes - they should have the same size.
• The HManager#createGraph method has been renamed to HManager#createBackendGraph - any reason for this?
• You have created a number of new files - could you add the usual Eclipse copyright header with the SPDX-License-Identifier header? We need it for the currently ongoing CQ.

Once this goes through, we should add a feature request for enabling/disabling plugins in remote Hawk instances.

[beatrizsanchez]

Hi Antonio,

api.dt now depends on ui2. Is there a reason for this? I've tried removing the dependency and everything seems fine.

I did this because of the extension point that makes the "Hawk Servers" View fall under the Hawk category. I removed it and didn't complain so I guess is not needed.

<extension
         point="org.eclipse.ui.preferencePages">
      <page
            category="org.hawk.ui.preferences.HawkPreferencePage"
            class="org.hawk.service.api.dt.ui.HawkServersPreferencePage"
            id="org.hawk.service.api.dt.page"
            name="Servers">
      </page>
</extension>

If I press "Start" on an instance in the "Instance Manager", the "Start"/"Stop" buttons are not updated. Same goes when I stop a running instance.

Updated in latest commit.

You have created a number of new files - could you add the usual Eclipse copyright header with the SPDX-License-Identifier header? We need it for the currently ongoing CQ.

Updated in latest commit. Not sure if I am missing any.

We are not using those human readable names for the query engines in the Query dialog.

Updated in latest commit.

The capitalization style in some of the plugins is different (e.g. "Modelio metamodel resource factory"). We should standardize to "Modelio Metamodel Resource Factory", and perhaps have a second look at the others.

Updated in latest commit.

The HManager#createGraph method has been renamed to HManager#createBackendGraph - any reason for this?

At some point I was confused with this, when we create a hawk instance with the UI dialog the label to choose the graph says "Backend". I renamed it to avoid my confusion but It could go back though it might make more sense to chance to createGraphDatabase as the interface (IGraphDatabase) and maybe update the UI label to say Database?

The buttons in the "Instance Manager" have different sizes - they should have the same size.

Updated in last commit.

Does it make sense to allow the updater plugin to be changed at all? The graph structure would be completely different.

I've removed the updater plugin table. It doesn't make sense but somehow I understood that it was a necessary option. I have a question: an index only uses an updater from what I understand, if so, would it make sense to change the updater checkbox table into a combo box?

It doesn't make sense to enable/disable query engine plugins for an instance. You simply use a query engine plugin to run a query on a graph. Those should be removed from the preferences page and any future "Advanced" tab in the creation dialog.

I've removed the query language plugin table. Is there a way we can identify relevant query mechanisms for the current index configuration instead of listing them all? For example if I have a time aware index which has to use greyback does it make sense to provide OrientDB SQL?

What happens if we have (meta)models indexed through some plugins, and those plugins get disabled? Do we have a defined behaviour for it?

Not that I am aware of.

I'd expect that the models would probably be deleted on the next sync, but we need to test and document it (and potentially give a warning to the user).

I agree, for the time being I've disabled the removal of plugins that happened when unchecking. We can go back to this when we have done more testing.

We seem to have one regression - it is not possible to change the plugins while creating a new Hawk instance, as it was possible before. Could we add back that option, perhaps as an "advanced" tab in the instance creation dialog as Dimitris suggested?

Updated in last commit.

The updater checkbox table looks awkward but if we change to a combo box it might look better.

By default, all plugins are disabled. This means that upon first use, the Hawk instance would not be able to index anything. I think the defaults should be flipped - we could probably enable all metamodel and model plugins, and leave all change listener plugins disabled by default.

See previous comment.

[agarciadom]

Thanks for your quick work! I think we should change the updater table to use radio buttons or a combo box, yes.

We do not have any API right now to detect if a query engine is compatible with a certain backend / updater. We may want to have that, but I think that should go to a separate issue - could you file that as an enhancement request?

[agarciadom]

Let me know when you change to the combobox and I'll try this out again :-).

[agarciadom]

Hi Betty - any updates on this?

[beatrizsanchez]

Hi Antonio, sorry for the delay. I had some issues trying to update the plugins when there are factory type changes (either local or remote). There is only a method listPlugins for remote instances from which I had to use a naming-convention workaround to figure out which were metamodel, model or graph change listener plugins. I think this is far from ideal. I could leave it for this pull request and change the workaround later because I am not that confident on how to extend the remote factory interface (and or thrift) to provide methods like listMetamodelPlugins, listModelPlugins and listGraphChangePlugins.

[agarciadom]

Thanks for the comment! I have added in b19c680c a new verb to the Hawk Thrift API called listPluginDetails(), which will give you a list of all the available plugins, with their readable names, unique identifiers and type (according to an enum).

I have also tweaked all Hawk plugin types so they implement a base IHawkPlugin interface, to unify the names of the methods wrt unique ID / human-readable name. Could you update your UI code to use those methods?

[beatrizsanchez]

Thanks for doing this Antonio! I have now successfully rebased my changes onto the latest commit on master. Thanks for adding the listPluginDetails() method, however, I have some issues regarding how to make use of it:

The listPluginDetails method is part of the Hawk.Client Thrift API therefore it is not accessible through the IHawkFactory interface nor the ThriftRemoteHawkFactory class, as the getClient() method is protected. I don't believe I should call the following snippet within the ui2 plugin to figure out which plugins are we dealing with, as it would require a dependency to the service.api plugin.

ThriftProtocol proto = ThriftProtocol.guessFromURL(location);
Hawk.Client client = APIUtils.connectTo(Hawk.Client.class, location, proto, new LazyCredentials(location));
client.listPluginDetails()

I think the IHawkFactory interface should provide the details through the addition of a List<HawkPlugin> listPluginDetails() method and that we should provide a generic HawkPlugin class or interface in hawk.core that is equivalent to the one in hawk.service.api that now depends on Thrift. What are your thoughts on this? Am I missing something?

[agarciadom]

Oh, I see the problem. In fact, the IHawkPlugin interface has all information except for the type of the plugin itself. I think I will move the plugin type enum from Thrift to the IHawkPlugin interface, and then expose a List from a new IHawkFactory#listPlugins() method.

The remote implementation will still return a list of IHawkPlugin implementations, but these will be simple data holders and will be devoid of any real functionality.

[agarciadom]

Done: IHawkFactory#listPlugins now returns a List<IHawkPlugin>. However, for local instances this is just null (you will have to rely on HManager, since LocalHawkFactory and TimeAwareHawkFactory are supposed to be OSGi-agnostic). Remote instances do return the expected list.

In short, you should try the factory's listPlugins method, and if that returns null then use HManager#getAvailablePlugins. Let me know how it goes!

(BTW, you will have to rebase your branch on top of the latest master: Github is reporting conflicts. See above.)

[agarciadom]

Hey, is this ready to try out?

[beatrizsanchez]

Hi Antonio, yes it is. I have added the combo box, and merged all our changes. Sorry I had to squash and force-push. Plugin updates are working fine depending on the type of factory. You can try it out.

[agarciadom]

I'm trying this out now. I have noticed a few minor niggles in the UI - for instance, the list of plugins was being updated every time anything was touched, and not just the factory / remote instance URL. I have fixed that, and I've improved a few strings here and there and added sorting of updaters and backends.

I am noticing some issue with the EMF model parser not being picked up on creation - looking into this now.

[agarciadom]

I have merged this into master, with a number of fixes and minor tweaks on top. I had to add query engines after all into the configuration. The indexer needs to have the query engines registered into it for computing derived features, after all :-).

Thanks for your work!

[beatrizsanchez]

Thanks Antonio! Glad it's been merged :)