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
Fix Imports When Attaching Visualization #1857
Conversation
@@ -129,7 +129,10 @@ class UpsertVisualisationJob( | |||
moduleName: String, | |||
expression: String | |||
)(implicit ctx: RuntimeContext): Either[EvalFailure, AnyRef] = { | |||
val maybeModule = ctx.executionService.findModule(moduleName) | |||
val context = ctx.executionService.getContext | |||
// TODO [RW] more specific error when the module cannot be installed |
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.
Returning more specific errors would require altering the protocol which is out of scope of this quick fix.
I think that for now the ModuleNotFound
should be enough as it is correct - the module could not be found, it is just not as specific (whether it could not be resolved or the download failed etc.). I suggest to add this later on.
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.
Please file an issue for this.
.../runtime/src/test/scala/org/enso/interpreter/test/instrument/RuntimeVisualisationsTest.scala
Show resolved
Hide resolved
@@ -129,7 +129,10 @@ class UpsertVisualisationJob( | |||
moduleName: String, | |||
expression: String | |||
)(implicit ctx: RuntimeContext): Either[EvalFailure, AnyRef] = { | |||
val maybeModule = ctx.executionService.findModule(moduleName) | |||
val context = ctx.executionService.getContext | |||
// TODO [RW] more specific error when the module cannot be installed |
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.
Please file an issue for this.
Pull Request Description
Fixes a bug introduced by new library logic - as we now only load the libraries on-demand, the
Standard.Visualization
library was not preloaded and the attach visualization command did not try to preload it.Now, when the visualization is attached, it ensures that its module is preloaded.
Important Notes
Checklist
Please include the following checklist in your PR: