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.

Sign up for GitHub

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

Jump to bottom

Provide standalone modelserver-client #65

Closed
Tracked by #153
tortmayr opened this issue Nov 15, 2021 · 1 comment
Closed
Tracked by #153

Provide standalone modelserver-client #65

tortmayr opened this issue Nov 15, 2021 · 1 comment
Assignees
@tortmayr
Labels
enhancement New feature or request
Milestone
0.8.0 Stable Release

Comments

@tortmayr
Copy link
Contributor

tortmayr commented Nov 15, 2021 •
edited
Loading

Currently the model-server client API is nested into the modelserver-theia package and cannot be used in a context without Theia.
AFAIK the model-server client doesn't have any direct Theia dependencies and could be provided in a separate package which facilitates reusing it in a non-Theia application e.g. a plain node back-end.

This is also a good opportunity to discuss the overall architecture of the model-server client.
Currently a simple, custom rest client is used under the hood. While this client suffices to cover the basic usecases it has a couple of downsides:

  • The simple rest client is only working In a node context which limits the reuseability of the model server client as a whole. From a technical point of view there is no real reason for this limitation. Both REST & Websocket work perfectly fine in the browser.
  • The simple rest client as no exception handling concept e.g. throwing an error if a response with status code >200 is received. This forces us to expose low-level response objects via the ModelServer Client API & delegates the exception handling to API uses. With a sophisticated exception handling concept the ModelServer API could directly return the concrete data objects instead of the low level response.
  • There is no way to unit test the ModelServer Client API because a mocking framework for the simple rest client would be required.

I think most of these issues could be resolved by switching to a well-established rest client library like axios. There are also mocking frameworks available for axios which means unit tests could be implemented relativity easy.

We need to be careful with the handling of websockets be able to use the model serve client in both node & browser context. An isomorphic implementation that works in both contexts should be used.

We (EclipseSource) intend to contribute this on behalf of ST Microelectronics.
@tortmayr tortmayr added the enhancement New feature or request label Nov 15, 2021
@tortmayr tortmayr self-assigned this Nov 15, 2021
@tortmayr tortmayr mentioned this issue Jan 5, 2022
Closed
tortmayr added a commit that referenced this issue Jan 5, 2022
@tortmayr
#65 Provide standalone modelserver-client
245b1b2 
- Move packages into dedicated `packages` dir
- Move downloading of theia plugins from root `package.json` to browser-app
- Introduce  root script for upgrading the fluent theia dependencies (`yarn ugprade:theia`)
- Upgrade effective Theia version  to 1.19.0 and swapt to ES2017 as default target.
Add mocha dependencies and add config

- Restructure repo and move packages and examples in dedicated directories
- Extract ModelServerClient from modelserver-theia into separate package
 Rework ModelServerClient API so that 
-- The API is iinline with the current Java API
-- The API returns concrete data objects instead of low level responses
-- Overrides are provided to retrieve typed results where suitable (#22)
-- The default impl uses  the well established axios  library for rest calls instead of a custom solution
-- The implementation is isomorph and can be used in both web  frontends and node backends.
-- Rework can clean up Subscriptions  API

- Provide generic mocha test setup
-  Add simple request tests for the modelserver client (using moxios)

Contributed on behalf of ST Microelectronics
tortmayr added a commit that referenced this issue Jan 5, 2022
@tortmayr
#65 Provide standalone modelserver-client
b415414 
- Move packages into dedicated `packages` dir
- Move downloading of theia plugins from root `package.json` to browser-app
- Introduce  root script for upgrading the fluent theia dependencies (`yarn ugprade:theia`)
- Upgrade effective Theia version  to 1.19.0 and swapt to ES2017 as default target.
Add mocha dependencies and add config

- Restructure repo and move packages and examples in dedicated directories
- Extract ModelServerClient from modelserver-theia into separate package
 Rework ModelServerClient API so that 
-- The API is iinline with the current Java API
-- The API returns concrete data objects instead of low level responses
-- Overrides are provided to retrieve typed results where suitable (#22)
-- The default impl uses  the well established axios  library for rest calls instead of a custom solution
-- The implementation is isomorph and can be used in both web  frontends and node backends.
-- Rework can clean up Subscriptions  API

- Provide generic mocha test setup
-  Add simple request tests for the modelserver client (using moxios)

Contributed on behalf of ST Microelectronics
tortmayr added a commit that referenced this issue Jan 5, 2022
@tortmayr
#65 Provide standalone modelserver-client
c876061 
- Move packages into dedicated `packages` dir
- Move downloading of theia plugins from root `package.json` to browser-app
- Introduce  root script for upgrading the fluent theia dependencies (`yarn ugprade:theia`)
- Upgrade effective Theia version  to 1.19.0 and swapt to ES2017 as default target.
Add mocha dependencies and add config

- Restructure repo and move packages and examples in dedicated directories
- Extract ModelServerClient from modelserver-theia into separate package
 Rework ModelServerClient API so that 
-- The API is iinline with the current Java API
-- The API returns concrete data objects instead of low level responses
-- Overrides are provided to retrieve typed results where suitable (#22)
-- The default impl uses  the well established axios  library for rest calls instead of a custom solution
-- The implementation is isomorph and can be used in both web  frontends and node backends.
-- Rework can clean up Subscriptions  API

- Provide generic mocha test setup
-  Add simple request tests for the modelserver client (using moxios)

Contributed on behalf of ST Microelectronics
@martin-fleck-at martin-fleck-at mentioned this issue Jan 25, 2022
Closed
13 tasks
ndoschek pushed a commit that referenced this issue Jan 25, 2022
@tortmayr
#65 Provide standalone modelserver-client (#69)
79b6e7b 
* #65 Provide standalone modelserver-client 

- Move packages into dedicated `packages` dir
- Move downloading of theia plugins from root `package.json` to browser-app
- Introduce  root script for upgrading the fluent theia dependencies (`yarn ugprade:theia`)
- Upgrade effective Theia version  to 1.19.0 and swapt to ES2017 as default target.
Add mocha dependencies and add config

- Restructure repo and move packages and examples in dedicated directories
- Extract ModelServerClient from modelserver-theia into separate package
 Rework ModelServerClient API so that 
-- The API is iinline with the current Java API
-- The API returns concrete data objects instead of low level responses
-- Overrides are provided to retrieve typed results where suitable (#22)
-- The default impl uses  the well established axios  library for rest calls instead of a custom solution
-- The implementation is isomorph and can be used in both web  frontends and node backends.
-- Rework can clean up Subscriptions  API

- Provide generic mocha test setup
-  Add simple request tests for the modelserver client (using moxios)

- Use @theia/core/shared/inversify dependency where applicable
- Fix Jenkinsbuild
- Fix eslint setup
-- Align eslint dependencies so that the are compatible with the node 12 version used in Theia.

Contributed on behalf of ST Microelectronics
@ndoschek
Copy link
Contributor

Fixed with #69

@tortmayr tortmayr mentioned this issue Jan 25, 2022
Open
tortmayr added a commit that referenced this issue Jan 25, 2022
@tortmayr
#65 Add typeguard functions for modelserver notifications
f84e8c7 
Add missing typeguard functions for modelserver notifications. To avoid unnecessary complex type checks the 'type' property is used as discriminator.
Follow up for #65
Contributed on behalf of STMicroelectronics
@tortmayr tortmayr mentioned this issue Jan 25, 2022
Merged
ndoschek pushed a commit that referenced this issue Jan 25, 2022
@tortmayr
#65 Add typeguard functions for modelserver notifications (#77)
6344e0e 
Add missing typeguard functions for modelserver notifications. To avoid unnecessary complex type checks the 'type' property is used as discriminator.
Follow up for #65
Contributed on behalf of STMicroelectronics
@CamilleLetavernier CamilleLetavernier mentioned this issue Feb 2, 2022
Merged
@martin-fleck-at martin-fleck-at added this to the 0.8.0 Stable Release milestone Feb 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tortmayr tortmayr

Labels
enhancement New feature or request
Projects
None yet
Milestone
0.8.0 Stable Release
Development

No branches or pull requests
3 participants
@tortmayr @martin-fleck-at @ndoschek