Skip to content
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

Refactor script dependency tracking #3168

Merged
merged 8 commits into from
Nov 29, 2022
Merged

Conversation

J-N-K
Copy link
Member

@J-N-K J-N-K commented Nov 20, 2022

Fixes #3166

This needs to be merged together with the corresponding PR in openhab-addons, otherwise jsscripting will break.

With this change every ScriptEngineFactory can provide a DependencyTracker that is injected in the execution context. Dependencies are tracked by an identifier (usually a path). Services implementing DependencyTracker.Listener are notified of the script that need reloading by the engineIdentifier of the script engine.

The new structure allows decoupling of script/rule providers (file-based / UI based) and dependency trackers.

Signed-off-by: Jan N. Klug github@klug.nrw

@J-N-K J-N-K added the bug An unexpected problem or unintended behavior of the Core label Nov 20, 2022
Signed-off-by: Jan N. Klug <github@klug.nrw>
@J-N-K J-N-K force-pushed the bug-dependencytracker branch from 3cc4606 to abc6042 Compare November 20, 2022 17:44
Signed-off-by: Jan N. Klug <github@klug.nrw>
Signed-off-by: Jan N. Klug <github@klug.nrw>
Signed-off-by: Jan N. Klug <github@klug.nrw>
Signed-off-by: Jan N. Klug <github@klug.nrw>
@J-N-K J-N-K marked this pull request as ready for review November 20, 2022 20:18
@J-N-K J-N-K requested a review from a team as a code owner November 20, 2022 20:18
Signed-off-by: Jan N. Klug <github@klug.nrw>
@J-N-K
Copy link
Member Author

J-N-K commented Nov 21, 2022

@openhab/core-maintainers If this should make it into 3.4 it should be reviewed ASAP. Even though I tested it for JS (and all other languages did not use dependency tracking, neither for file-based nor UI scripts/rules), there is the slight risk that it may break something. The broken API probably is not an issue since TTBOMK it was not used (except in the GraalJS add-on).

Copy link
Member

@wborn wborn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a useful addition to me! I've added a few comments below:

Signed-off-by: Jan N. Klug <github@klug.nrw>
Signed-off-by: Jan N. Klug <github@klug.nrw>
Copy link
Member

@wborn wborn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much. LGTM now!

@wborn wborn merged commit 4bcc15d into openhab:main Nov 29, 2022
@wborn wborn added this to the 3.4 milestone Nov 29, 2022
@J-N-K J-N-K deleted the bug-dependencytracker branch November 29, 2022 20:06
splatch pushed a commit to ConnectorIO/copybara-hab-core that referenced this pull request Jul 12, 2023
Signed-off-by: Jan N. Klug <github@klug.nrw>
GitOrigin-RevId: 4bcc15d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API breaking bug An unexpected problem or unintended behavior of the Core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[automation] Missing dependency listener/tracker for UI scripts
2 participants