Fixes scripts not updating on change in development mode #245
Conversation
MSattrtand
changed the base branch from
main
to
231-improve-query-parse-error-logging
MSattrtand
changed the base branch from
231-improve-query-parse-error-logging
to
main
MSattrtand
force-pushed
the
227-fixes-scripts-not-updating-on-change
branch
from
de19731 to
5c827bd
Compare
blcham
force-pushed
the
227-fixes-scripts-not-updating-on-change
branch
from
79cc0dd to
bb83ef1
Compare
| @@ -88,7 +88,7 @@ public Module loadModule(final String moduleId, | |||
|
|
|||
| // TODO check moduleTypeUri | |||
|
|
|||
| return PipelineFactory.loadPipeline(scriptsRepository.getResource(resourceUri, resourceContextUri)); | |||
| return PipelineFactory.loadModule(scriptsRepository.getResource(resourceUri, resourceContextUri)); | |||
There was a problem hiding this comment.
I am not sure that this is ok, i will check
There was a problem hiding this comment.
loadPipeline() loads the script from the saved pipeline, not from the file we've changed, if I remember correctly. So, it will use the old version of the script.
There was a problem hiding this comment.
Here some detail is needed to understand if we need to refactor or it is reasonable change.
blcham
force-pushed
the
227-fixes-scripts-not-updating-on-change
branch
from
bb83ef1 to
9105376
Compare
| @@ -358,7 +362,11 @@ Model loadModelFromUrl(@NotNull String modelUrl) { | |||
| private void extendBindingFromURL(VariablesBinding inputVariablesBinding, URL inputBindingURL) { | |||
| try { | |||
| final VariablesBinding vb2 = new VariablesBinding(); | |||
| vb2.load(inputBindingURL.openStream(), FileUtils.langTurtle); | |||
| String path = inputBindingURL.getPath(); | |||
There was a problem hiding this comment.
I do not understand why you are doing this ... and believe there will be inputstream that we will never close
There was a problem hiding this comment.
If we are using old method, so vb2.load(inputBindingURL.openStream(), FileUtils.langTurtle);, we'll get exception
java.net.ConnectException: Connection refused: getsockopt
E. g. if we want to rerun construct-greeting, we'll get:
We do not have id or is-greeted-by-message, unlike here with my changes:
So, I'm loading the pipeline another way since it is saved on a disc.
| @@ -221,7 +219,13 @@ private Model runModule(final Model inputDataModel, final MultiValueMap<String, | |||
|
|
|||
| ExecutionEngine engine = createExecutionEngine(configModel); | |||
| ContextLoaderHelper.updateContextsIfNecessary(scriptManager); | |||
| Module module = PipelineFactory.loadModule(configModel.createResource(id)); | |||
| Module module = null; | |||
| if (scriptManager.doesRegistryContainsEntity(id)) { | |||
There was a problem hiding this comment.
Why using once scriptManager and then PipelineFactory?
There was a problem hiding this comment.
I'm trying to load it from the scriptManager if the required module exists there. If it does not, e.g. we have deleted it from the file. In this case, I'm trying to load it from the saved pipeline (so it is the last saved instance before deletion), so we would have at least some working module. I'm not sure if it is the best solution; maybe it would be better to let the code throw an exception in
if (module == null) { throw new SPipesServiceException("Cannot load module with id=" + id); }
In case we are trying to load a non-existing module
There was a problem hiding this comment.
Also, deleting an attempt to load with PiplelineFactory will cause test failures, so the tests should be changed in this case
Resolves #227