[Automation] Add DynamicImport-Package for scripted automation

[5iver]

Fixes #610. Discussed in #653.

Signed-off-by: Scott Rushworth openhab@5iver.com (github: openhab-5iver)

[5iver]

It looks like DCO does not accept the signoff, if you include the GH account.

[maggu2810]

AFAIK a github username is not (and has never been) part of the common signed off footer at all.

Is there a defined syntax or is the one choosen by the Git command line tool a suggestion only?

[kaikreuzer]

AFAIK a github username is not (and has never been) part of the common signed off footer at all.

For openHAB, it has always been, see https://www.openhab.org/docs/developer/contributing/contributing.html#sign-your-work.

But I'd be fine to drop this requirement and go with the "plain" sign-off.

[5iver]

@maggu2810, is this approach for resolving the issue still in question? I was hoping this could be merged before you left for holiday. 😄

[maggu2810]

I still would like to get some more comments about the dynamic imports from the other maintainers.

[wborn]

Dynamic imports look enticing at first. But after a bit of reading my conclusion is also that using dynamic imports is not recommended (causing issues) and it is regarded as a sign of a non-modular architecture.
It should only be used as a last resort. So this solution might cause further stability and performance issues with the scripting engine. I'd rather have more stability/performance instead of being able to load any class in the world.

[davidgraeff]

The referenced document is indeed speaking against dynamic imports with strong arguments.
What are our alternatives though for a dynamic environment like scripting? We are basically excluding script users from using public but not scriptengine exposed java classes.

[5iver]

@davidgraeff, thank you for stirring up the conversation! IMO #610 should be labelled as critical, since nobody that uses scripted automation can use any of the builds since ESH was reintegrated.

The alternative is to use Markus' solution in #653, which requires manually adding all imports, which I believe could all be optional. In testing this worked, but IMO is unnecessarily restrictive and will require upkeep to expose more Classes for use in scripts. The articles I have found in regards to OSGi and JSR223, suggested using DynamicImport. Of the articles recommending against DynamicImport, I could not see how the concerns that were being illustrated applied to OH... but I conceded to the experts!

@kaikreuzer, could you please weigh in on this?

[lewie]

Thanks @openhab-5iver for this Comment:
#653 (comment)

https://www.vogella.com/tutorials/OSGi/article.html#life-cycle-of-plug-ins-in-osgi

For legacy reasons OSGi supports a dynamic import of packages. See OSGi Wiki for dynamic imports for details. You should not use this feature, it is a symptom of a non-modular design.

This state is the same since more than 10 years and you're right, we have to know about the dangers for the dynamic processes in OSGI. I guess there is a reason why this functionality is not set to deprecated. Because exactly here we have an exemplary case in which the many advantages outweigh.

If scripting binding is not started, no danger. If binding is started and no no special class are loaded, no static dependencies will be created, no danger. We have all been able to live with this very well so far, haven't we?

And yes, this circumstance should definitely be emphasized in the documentation. But please don't forbid it completely!

It helps to test many things while scripting and I use the dynamic class-loading feature many many times!

Edit:
If problems arise in practice, one could raise the hurdle a little higher:
For example, a set of classes that is provided statically.
Additionally, in the binding configuration you could offer the possibility to load a small additional binding, which turns on the dynamic import of packages.

[CrazyIvan359]

Just thought I'd give my 2 cents here. I've been using openHAB for 2.5 years now and over the last few months I've been moving my automation over to Jython. I think this is a great benefit for openHAB as a whole to move away from DSL rules.

That said, I'm not a novice when it comes to programming and I am already using imports from all over the core. Because of this I am now stuck back on an older build until this is resolved. I've been following along with @openhab-5iver 's progress getting this sorted out and have a suggestion that might appease both sides: What if we used dynamic imports, but allowed only classes from the core and utilities to be imported?

Certainly we would have issues if a class that no longer exists or changed names was being imported by a script, but the core and utilities (with the exception of transform) don't change a runtime very often. Using dynamic import to get a class from a binding would be more likely to cause issues, but this could be denied.

I am not a Java programmer and have not used OSGi before, so there's a chance I've misunderstood dynamic import and this filtering is not possible

[wborn]

We have all been able to live with this very well so far, haven't we?

I thought it was newly added but it indeed seems to have been used before (see ESH manifest). Were there any reasons for this @maggu2810?

[davidgraeff]

I have thought about in more detail and we already now have a quite confusing versioning (non semantic) / non consistent API (java/rest/scripts) and basically no existing ABI.

I'd say we go with a third way next to Markus static way of listing all classes and the all in solution.

I propose to annotate all classes that are going to be exported. An OSGi bundle tracker in the automation.script bundle finds all annotated classes and makes them available.

That way a developer immediately sees that an "API contract" with the script engine is established and changes could potentially break user setups.

[5iver]

I propose to annotate all classes that are going to be exported.

Interesting, but this does not help with third party libraries, so would need to be used in conjunction with manual imports. Here are three examples.

I thought it was newly added but it indeed seems to have been used before

DynamicImport was included in the original PR for scripted automation, so if there are any ill effects from using it, we should already be aware of them. I have no doubts that the reason it was added in the first place was to provide exactly the functionality that we are asking to be restored... access to all classes for use within scripted automation.

[davidgraeff]

@openhab-5iver I see the point. But I'm also asking myself, how do you inject 3rd party java libraries as a script developer? Wouldn't you instead use script engine provided ways and 3rd party script libraries?

I think what is problematic for Markus and me is that a script user can access all framework classes and we would need to care about them. And framework classes definitely need an overhaul over the next months when we are allowed to break the API.

[lewie]

@davidgraeff

I think what is problematic for Markus and me is that a script user can access all framework classes and we would need to care about them.

I don't know how you could do that with these many different possibilities and languages that "free" script users use! And I don't see the need for that either. If the API changes, then we have to add this changes to the textual scripts, that's the way it is. Of course we should coordinate together, as we are doing right now.

I think we have to distinguish between the scripts compiled in the PaperUI and the free scripts that are used for testing purposes and innovations. The scripts in the PaperUI have to work relatively without any special attention to conventions. But I don't expect that from the free textual scripting part.

Therefore my suggestion to have to activate the dynamic import of packages for experienced users first. So the not so experienced user does not get tangled up in the conventions of core java classes.

At least that's how I think both approaches to scripts would be possible?

[5iver]

But I'm also asking myself, how do you inject 3rd party java libraries as a script developer?

The Jython setup includes a directory that is added to the classpath, where you can add external libraries. At runtime, you can also add jars using sys,path.append. I do not know if/how this can be done with the other scripting languages.

I'm not sure if this is being suggested, but if OH does not make these classes available to scripted automation, the jars can still be added and the classes imported (at least for Jython). The libraries can be updated so that people's scripts work again. However, there are plenty of people who don't use the libraries, including people using scripted Actions/Conditions, that will need to make these updates on their own. So, it's possible, but VERY time consuming and tedious, and even more prone to breaking due to changes.

It would be different if this was in preparation for OH3, but for now, IMO we should just put it back the way it was. If this is functionality that is going to be pulled in OH3, we can plan for it. Moving forward, pretty much all of the funcitonality provided by the helper libraries will be implemented as ModuleTypes and/or a scripting API. But even after that, my preference is to still leave the door open for experimentation and access to functionality where an abstraction layer has not yet been built.

For a few days now, my production system has been on S1557 with an updated org.openhab.core.automation.module.script which includes this PR. After a few library tweaks, automation has been running like it was before bnd.

[vbier]

It would be nice if this would be fixed in the short term. I am waiting on a fix with my not working groovy rules. Imho, you should just fix this with dynamic imports asap (to restore functionality for people like me) and then implement a scripting api later on.

[maggu2810]

My understanding has been that the package name changes of the automation part has been already a "big" breaking change and it is now a good time to address such technical stuff.
If we do not want to break additional things we could enable dynamic imports again and break your stuff again later.
Is there a notice (I assume I already asked it before and perhaps it has been already answered) in an official documentation (now) that no API that is used by scripting will be stable and is allowed to be changed in every build? You argue now that scripting is good for experiments and testing and if something is changes and broken it is okay. But I think we should be prepared for issues that complains about broken scripts and we should not change core part, drop Joda, ... because some script is using it.

[vbier]

The problem with this only is that nothing works as long as you have come up with a nice solution.

Adding dynamic imports only restores the former state and can easily be reverted later on. Do you really think it is better for users to wait for a working version for a month without JSR rules instead of fixing the package names now and then adapt the rules to a new API in a month?

[maggu2810]

Do you really think it is better for users to wait for a working version for a month without JSR rules

@vbier Haven't I already comment that we could add dynamic imports now and break things later?
So, what differs from my comment to your suggestion?

[vbier]

Nothing, I just misread what you wrote. Adding dynamic imports now and then come up with a nice scripting api would be a good solution.

[davidgraeff]

@maggu2810 You need to press the "merge" button :D

[maggu2810]

Reviewers
No reviews—at least 1 approving review is required.

I will add my review and agree now, but it would be always nice if other maintainers use the review feature, too. At least for such decisions.

[openhab-bot]

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/mqtt-2-binding-publish-action-with-jsr223-javascript/71417/6

[5iver]

Is there a notice (I assume I already asked it before and perhaps it has been already answered) in an official documentation (now) that no API that is used by scripting will be stable and is allowed to be changed in every build?

@maggu2810, I've added a warning into the JSR223 docs, and will add another in the openhab2-jython repo. Hopefully this addresses your immediate concern. Once #635 is merged and openhab2-addons/4801 is wrapped up (and another for Groovy), I will focus on rule Actions and a scripting API.

[openhab-bot]

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/jsr223-jython-simplified-rule-definition-similar-to-rules-dsl-and-universal-decorator/53252/158

[openhab-bot]

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/latest-snapshot-that-runs-jython-properly/73880/2