-
Notifications
You must be signed in to change notification settings - Fork 59
Use custom class loaders to load Languagetool dependencies #2248
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
Conversation
…s context menu, auto complete etc. and make the UI similar to the javafx TextArea
…able/disable spell checking
…ng, workaround to fix the slowness in initial spellcheck etc.
# Conflicts: # CorePluginFramework/src/au/gov/asd/tac/constellation/plugins/parameters/PluginParameter.java
# Conflicts: # CoreDataAccessView/src/au/gov/asd/tac/constellation/views/dataaccess/plugins/experimental/TestParametersPlugin.java # CorePreferences/src/au/gov/asd/tac/constellation/preferences/ApplicationOptionsPanel.form # CorePreferences/src/au/gov/asd/tac/constellation/preferences/ApplicationOptionsPanel.java # CoreWhatsNewView/src/au/gov/asd/tac/constellation/views/whatsnew/whatsnew.txt
…ected previously and improvements.
# Conflicts: # CorePreferences/src/au/gov/asd/tac/constellation/preferences/ApplicationOptionsPanel.form # CorePreferences/src/au/gov/asd/tac/constellation/preferences/ApplicationOptionsPanel.java # CorePreferences/src/au/gov/asd/tac/constellation/preferences/ApplicationOptionsPanelController.java # CorePreferences/src/au/gov/asd/tac/constellation/preferences/Bundle.properties # CoreWhatsNewView/src/au/gov/asd/tac/constellation/views/whatsnew/whatsnew.txt
…l and main app * add a message explaining the error or when no suggestions available
# Conflicts: # CoreDependencies/nbproject/project.xml # CoreDependencies/src/ivy.xml # CoreWhatsNewView/src/au/gov/asd/tac/constellation/views/whatsnew/whatsnew.txt
… conflicts using "Resolve using 'theirs'"
# Conflicts: # CoreDataAccessView/src/au/gov/asd/tac/constellation/views/dataaccess/plugins/experimental/TestParametersPlugin.java # CoreDependencies/src/ivy.xml # CorePluginFramework/src/au/gov/asd/tac/constellation/plugins/gui/PasswordInputPane.java # CorePluginFramework/src/au/gov/asd/tac/constellation/plugins/gui/ValueInputPane.java # CorePreferences/src/au/gov/asd/tac/constellation/preferences/ApplicationOptionsPanel.form # CorePreferences/src/au/gov/asd/tac/constellation/preferences/ApplicationOptionsPanel.java # CorePreferences/src/au/gov/asd/tac/constellation/preferences/Bundle.properties # CoreWhatsNewView/src/au/gov/asd/tac/constellation/views/whatsnew/whatsnew.txt
# Conflicts: # CoreWhatsNewView/src/au/gov/asd/tac/constellation/views/whatsnew/whatsnew.txt
|
To reuse the common transient libraries I let ivy resolve all dependencies in the same location, which avoids duplication. Then in the code I am keeping a list of the jars used by Languagetool in a text file and load those jars from the common location, ignoring the version numbers. The conflicting library |
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.
Its working well, just a few minor changes
...ilities/src/au/gov/asd/tac/constellation/utilities/text/LanguageToolDependencyUrlLoader.java
Show resolved
Hide resolved
CoreUtilities/src/au/gov/asd/tac/constellation/utilities/text/LanguagetoolClassLoader.java
Show resolved
Hide resolved
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.
There seems to be a configuration issue when trying to build constellation-adapters with the spellcheck version of constellation as the base.
:: resolving dependencies :: au.gov.asd.tac#third-party-dependencies;working@ZZ-AABBCCDD
confs: [defaultconf]
:: problems summary ::
:::: ERRORS
requested configuration not found in au.gov.asd.tac#third-party-dependencies;working@ZZ-AABBCCDD: [ 'defaultconf' ]
Logged #2295 to fix the Undo after Ignore all |
Fixed constellation-adapters in constellation-app/constellation-adaptors#39 |
Also added a |
It it working well but in testing I found that it was making the Data Access View take almost a whole minute to open which is not ideal. This happened both with and without Adaptors built. Doing the same steps on v3.1 only took about 3 seconds to open the Data Access View |
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.
Minor but I think it might be worth adding a TODO comment to some of the custom class loader parts of the code, just to acknowledge that this is a workaround which can be removed once the dependency clash is no longer an issue. Having a reference in code will make it easier to track than remembering this PR
CoreUtilities/src/au/gov/asd/tac/constellation/utilities/text/SpellChecker.java
Outdated
Show resolved
Hide resolved
I can recreate the slowness ~30-50s in the spellcheck branch. But it is quite random and I cannot pinpoint anywhere. The logs suggest the spellchecking actions are performed in less than a second. (Can we rely on time stamps?) When I could recreate, a considerable time was taken between discovering data access plugins: In fact when I merge the spellcheck branch into master, it's consistently faster. As Delphinus8821 suggested we should merge and see the performance in a built version before making any more effort? I've removed the unnecessary while loop, although it didn't affect the slowness. |
|
Prerequisites
Reviewed the checklist
Reviewed feedback from the "Sonar Cloud" bot. Note that you have to wait
for the "CI / Unit Tests") to complete first. Failed Unit tests can be
debugged by adding the label "verbose logging" to the GitHub PR.
Description of the Change
Custom class loaders are now used to load Languagetool dependencies, to avoid indriya version conflicts.
languagetool requires indriya v1.3 or earlier while systems-common used by gt-opengis is not compatible with 1.3 and requires a later version. (v2.0.2 or latest v2.2) This caused #2054
All other approaches I've tried, like ivy configs, Maven Shade Plugin and Relocation, only works for build-time conflicts. In our scenario we need to isolate 2 versions of the same library at runtime. Using a Custom ClassLoader seems to be the only possibility.
I used ivy configs to isolate the languagetool libs into a separate folder, rather than listing all the transient dependencies of languagetool in the code. The latter could cause version conflicts, as Ivy might resolve to different versions than those we have hard-coded. On the other hand ivy configs duplicates the common transient libraries.
I am loading only the Languagetool dependencies separately, keeping the rest of the constellation intact, to avoid too much complications, and when the Languagetool developers fix their issue we can easily revert the changes. (Usually both versions of the dependency is loaded using custom class loaders)
Know issues:
Undo after Ignore all doesn't work (Pre-existed) [Logged #2295 ]
To Do:
Find a way to reuse the common transient libraries.
Alternate Designs
Why Should This Be In Core?
Enables spell checking in enabled text fields
Benefits
Possible Drawbacks
Complex coding
Verification Process
Applicable Issues
#870