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

More intuitive RestApplication options object #6201

Closed
hacksparrow opened this issue Aug 26, 2020 · 3 comments
Closed

More intuitive RestApplication options object #6201

hacksparrow opened this issue Aug 26, 2020 · 3 comments
Labels
developer-experience Issues affecting ease of use and overall experience of LB users

Comments

@hacksparrow
Copy link
Contributor

Currently, the RestApplication options object look like this:

new RestApplication({
  rest: {
    port: 3001,
    host: '127.0.0.1'
  }
});

It should look like this:

new RestApplication({
  port: 3001,
  host: '127.0.0.1'
});

When we already know it is a REST application, we shouldn't have to add a rest property again to the options object, it is not intuitive.

If, for any reason, we can't bring the server options to the top level, rest should be named too server to be more accurate about what we are dealing with.

new RestApplication({
  server: {
    port: 3001,
    host: '127.0.0.1'
  }
});
@hacksparrow hacksparrow added the developer-experience Issues affecting ease of use and overall experience of LB users label Aug 26, 2020
@dhmlau
Copy link
Member

dhmlau commented Aug 26, 2020

I think there was a discussion a while back that one Application can have multiple servers, probably that's why we have this configurations.
cc @raymondfeng

@raymondfeng
Copy link
Contributor

I would like to keep the application config consistent for all Application classes. There are other common properties that can be configured for different types of applications, such as shutdown hooks. Using RestConfig for RestApplication will lose such capabilities.

@hacksparrow
Copy link
Contributor Author

I think there was a discussion a while back that one Application can have multiple servers, probably that's why we have this configurations.

@dhmlau yes, in case of Application it is understood. However, here we are specifically dealing with RestApplication, which cannot have any more servers. At the same time, I can very well understand where @raymondfeng is coming from.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
developer-experience Issues affecting ease of use and overall experience of LB users
Projects
None yet
Development

No branches or pull requests

3 participants