-
-
Notifications
You must be signed in to change notification settings - Fork 392
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Install JS Scripting (GraalJS) by default #1455
Conversation
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Therefore it needs to be installed by default.
Not sure if this has been discussed anywhere already, but imho the JS scripting is an "add-on" and should therefore never be installed by default (otherwise it should rather be a core bundle).
The users should have the choice, which scripting option to use with openHAB.
Since JS scripting has a whopping 26MB, it blows up any openHAB installation significantly, which will be an issue for lower end hardware.
I'd therefore rather think that the Blockly editor should check whether JS scripting is installed and if this is not the case either (a) not be available itself in the Main UI or (b) ask the user to install the JS scripting add-on before being able to use Blockly.
I have an open PR that removes some unneeded dependencies (they were required in the past), but that unfortunately doesn't seem to noticably reduce the bundle size. I guess we then need to ask the user to install JS Scripting to use Blockly. |
But if we don't install it by default, at least the openHAB demo server needs it for the Blockly demo. |
Installing it on the demo server is no problem at all. |
I agree with @kaikreuzer and that is also the reason why I proposed openhab/openhab-core#3233 (but that is not so easy, I would put that to the bottom of my priority ATM). |
For the record: Blockly 4.0 will depend either on Nashorn or GraalVM/JSScripting - it's support both options but if I understand you right, you don't want to have any of both preinstalled? I never though about not having any Javascript installed at all, so I never got the idea of checking that. Maybe someone jumps in to generate DSL with Blockly ;-) |
But what about rrd4j? That's an add-on and it's installed by default in a fresh OH 3 install. We should be consistent. The same arguments for having rrd4j available by default (i.e. to make a core feature of MainUI work out-of-the-box). Both should be treated the same. I don't think anyone is arguing that JS Scripting can't be uninstalled not would I expect it to just be installed out of the blue on an upgrade from 3 to 4. But if rrd4j can be installed on a new install, the same reasoning should apply to including any other add-on by default. The size issue is separate and reason enough to not install JS Scripting by default. But all the other mentioned reasons should add up to rrd4j not being included too. As for when/if Rules DSL becomes it's own add-on and/or rrd4j is excluded we have to consider the fact that we will ship a default instance that's incomplete. Though maybe the wizard in MainUI can be used to pick and choose these missing parts from among the options. |
I was thinking something similar, openHAB will not have a rules engine installed by default. I think it makes sense to keep these add as addons, but how do we include a sane number of add-ons that makes the system useful out of the box, either in the distro or somewhere else? I know the WebUI has a walk through "first run" menu that prompts the user to install bindings, maybe this could be extended to specifically call out a rules and persistence binding? (so maybe suggest JSScripting & RRD, but let the user select others?) Edit: |
Forgot to mention @ghys in my last post ☝️ |
Good point, you are clearly right that this is an exception to the rule - but it shouldn't become an archetype for it. ;-)
@stefan-hoehn With openhab/openhab-core#3233 in place, there would not even be the Rule DSL installed by default.
This is also a solution I had in mind. The setup wizard would imho be the right place to tell the user that he needs to decide for a scripting (and possibly a persistence) option and present him the choices. So we would be sure that after finishing the setup wizard, the user would end up with a "complete" installation. |
Out of curiosity, what hardware are you concerned about? I totally get if every bundle was 25+ MB , the distro would be huge and not useable on common hardware like PI's and Beagle Boards, but a rule engine seems to be worth an exception (you usually don't need more then one installed). 26mb then does not seem that large to me, even on a Raspberry PI 3. |
Another question.. Does this bundle need to be installed in order to use the replacement for the Javascript transformation? If so, that would argue for installing it by default, because it's not obvious that that's what you need to do if you were using the Javascript transformation. |
Sounds reasonable to me, we shouldn't miss the script editor itself and also present an installation option there.
Yes and no. The replacement for JavaScript transformation is to use the SCRIPT transformation and have a JS Automation Addon installed. You either need JS Scripting (GraalJS) or JS Scripting (Nashorn).
With a note in the changelog, the docs and the UI as well I think it would be obvious. |
I don't think I meant to say that. What I was meaning is that rrd4j is required to get full functionality for MainUI. Similarly, since Blockly is a part of MainUI, the same arguments for including Nashorn or JS Scripting should apply, or the same arguments that exclude them should also apply to rrd4j.
You need some automation add-on installed. The SCRIPT transform supports them all, not just JS. You theoretically could write a transform in Blockly, though I don't think there is the kind of support in MainUI to allow that yet. So I wouldn't say we need Nashorn or JS Scripting installed to support transforms, just that one needs to be installed, any one.
We area mere couple of weeks into OH 4 and I'm already thinking we will need a migration tutorial like we had between OH 1 and OH 2. Overall the changes are not as drastic but there are some fundamental differences that I think we'll need a mapping for lots of users. The intro of the SCRIPT transform is merely one issue that will need detailed step-by-steps. We have problems identified with running on Debian Buster and Java 17, potentially significant changes to the handling of UoM, Item metadata, etc. OH 4 is looking to be an exciting release! :-D |
I was thinking the same thing a while ago - adding some steps to the setup wizard.
Could be done in the list of languages you get when creating a script/script module. The case when there's none hasn't been foreseen (as Nashorn was always available) but it could be handled with some nice message and a link instead of showing the list (and preventing to go forward). There's no need to check for the presence of the add-on every time the script editor is opened IMO, that's an unnecessary API request. |
Maybe under developer tools we could add an option to run the wizard again? |
So how do we want to proceed here? |
My reading of the consensus is that this should be handled through the first start wizard (and docs). I'd move rrd4j to the wizard too for consistency but that's just me it seems. |
Agreed.
I agree here as well - makes it consistent indeed. In any case, this PR can and should be closed. |
Is there an issue over on webuis to capture this needs to be done in the wizard? |
No there is no issue. I'll create one. |
WebUI issue is created, see openhab/openhab-webui#1659. |
openhab/openhab-webui#1659 added recommended addons to the setup wizard. These will get installed on a new openHAB install if the user does not actively de-select them, so the RRD4J installation from the distro is not required anymore. See also openhab#1455. Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
openhab/openhab-webui#1659 added recommended addons to the setup wizard. These will get installed on a new openHAB install if the user does not actively de-select them, so the RRD4J installation from the distro is not required anymore. See also openhab#1455. Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
…izard (#1543) * Remove RRD4J from default installation * Remove standard package (default installation) openhab/openhab-webui#1659 added recommended addons to the setup wizard. These will get installed on a new openHAB install if the user does not actively de-select them, so the RRD4J installation from the distro is not required anymore. See also #1455. Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
With openhab/openhab-webui#1597 getting resolved soon, Blockly will depend on JS Scripting (GraalJS). Therefore it needs to be installed by default.
@wborn Ping.