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

Modules install resource #5

Merged

Conversation

gianluca-nitti
Copy link
Member

This small pull request adds the feature which allows server administrators to install modules in the server using the web interface or API.

The added resource, ModuleInstallerResource:

  • implements WritableResource<Name[]> in the sense that you can write to it (i.e. with a POST HTTP request or with the appropriate WebSocket message) a list of modules you want to install (which does not need to include all the dependencies; the dependency resolution happens server-side and the server adds the required dependencies - if any - to the download list automatically)
  • extends ObservableReadableResource<String> in the sense that you can read from it the current status of the installation, and every connected WebSocket client is automatically notified when the status changes (i.e. a progress in the download is made, or the installation has been completed).

Important: for this to work, the engine must be built including my commits on pull request MovingBlocks/Terasology#3028; FacadeServer won't compile if classes introduced in that PR, like ModuleInstallManager, are missing.

@gianluca-nitti gianluca-nitti added this to Pull requests in review in GSOC 2017 - Server API Aug 7, 2017
@gianluca-nitti
Copy link
Member Author

Update: while doing some additional testing I noticed that the WebSocket connection between the server and the web interface was frequently dropping when using this feature. After researching a bit of information about the exception that was being thrown by some Jetty internal class, I found out that there was a concurrency issue - multiple threads sometimes were trying to send WebSocket messages at the same time (when the write method of a WritableResource returns, an ActionResult whose content depends on whether the method threw a ResourceAccessException or not is sent to the client, but inside that method ModuleInstallerResource starts a parallel task to download the new modules which can send messages to report progress), and the previously used method didn't support this.
So I solved this issue in the latest commit by simply switching to another override of the sendString method of the RemoteEndpoint Jetty class; the currently used version is non-blocking, returns immediately and then calls a callback after the message has been sent. Looks like everything works correctly now. The method's javadoc isn't very exhaustive but the comment in the Jetty bug tracker talks about an internal buffer so in my understanding the messages are queued and then sent once at a time to the underlying socket.

@msteiger
Copy link
Member

msteiger commented Aug 8, 2017

Great find about the Jetty "deficiency".

@Cervator
Copy link
Member

Engine PR has been merged, I think this one can be as well now :-)

@msteiger
Copy link
Member

Thanks @Cervator and @gianluca-nitti of course.

@msteiger msteiger merged commit 3908a3b into MovingBlocks:develop Aug 15, 2017
@gianluca-nitti gianluca-nitti moved this from Pull requests in review to Merged/completed in GSOC 2017 - Server API Aug 15, 2017
gianluca-nitti added a commit to gianluca-nitti/FacadeServer that referenced this pull request Aug 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
GSOC 2017 - Server API
Merged/completed
Development

Successfully merging this pull request may close these issues.

None yet

3 participants