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

[feat] create additional http servers #36804

Merged
merged 20 commits into from
May 29, 2019

Conversation

toddself
Copy link
Contributor

@toddself toddself commented May 21, 2019

allow for additional http servers to be created, tracked and returned

Summary

This allows kibana plugins to create an additional http server with the
same options as the main server, but listening on a different port.

Closes #36713
Blocked by #37118

Dev Docs

New Platform plugins can now create additional HTTP servers on additional ports. Example:

class MyPlugin {
  setup({ http }) {
    this.additionalServer = http.createNewServer({ port: 3000 });
    this.additionalServer.registerRouter(new Router());
  }

  async start() {
    await this.additionalServer.start();
  }
}

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@toddself toddself requested a review from a team as a code owner May 21, 2019 17:21
@toddself toddself added the Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc label May 21, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform

@eliperelman
Copy link
Contributor

In order for this PR to automatically close the mentioned issue #, you will need to put this in the summary. Also, could you add back in the rest of the information from the pull request template?

Copy link
Contributor

@eliperelman eliperelman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just have a couple small requests.

src/core/server/http/http_service.ts Outdated Show resolved Hide resolved
src/core/server/http/http_service.ts Outdated Show resolved Hide resolved
@elasticmachine
Copy link
Contributor

💔 Build Failed

@toddself toddself requested a review from eliperelman May 21, 2019 20:31
src/core/server/http/http_service.test.ts Outdated Show resolved Hide resolved
src/core/server/http/http_service.ts Outdated Show resolved Hide resolved
src/core/server/http/http_service.ts Outdated Show resolved Hide resolved
src/core/server/http/http_service.ts Show resolved Hide resolved
@toddself toddself requested a review from eliperelman May 21, 2019 22:05
@toddself toddself force-pushed the multiple-http-service branch from 94a231b to 22f962a Compare May 21, 2019 22:41
@toddself toddself requested a review from joshdover May 21, 2019 22:42
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@toddself toddself force-pushed the multiple-http-service branch from 22f962a to 60e072a Compare May 22, 2019 15:06
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@toddself toddself mentioned this pull request May 22, 2019
13 tasks
Copy link
Contributor

@eliperelman eliperelman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple minor changes, my review is approved once those are done.

src/core/server/http/http_service.mock.ts Outdated Show resolved Hide resolved
src/core/server/http/http_service.ts Outdated Show resolved Hide resolved
src/core/server/http/http_service.ts Outdated Show resolved Hide resolved
src/core/server/http/http_service.ts Outdated Show resolved Hide resolved
src/core/server/http/http_service.ts Outdated Show resolved Hide resolved
src/core/server/http/http_service.ts Outdated Show resolved Hide resolved
src/core/server/http/http_service.ts Outdated Show resolved Hide resolved
src/core/server/http/http_server.ts Outdated Show resolved Hide resolved
src/core/server/http/http_service.test.ts Show resolved Hide resolved
@toddself toddself force-pushed the multiple-http-service branch from 60e072a to 875e7e4 Compare May 23, 2019 17:57
@elasticmachine
Copy link
Contributor

💔 Build Failed

@toddself toddself force-pushed the multiple-http-service branch from d779294 to 904be16 Compare May 24, 2019 17:22
toddself added a commit to toddself/kibana that referenced this pull request May 24, 2019
In elastic#36804 we need to validate a `Partial<HttpConfig>` object which may
or may not have all the required keys attached to it.  This is the type
of use-case for `Joi.reach`, so we should expose a reach-like method on
the object validation class so that you can return the validator for a
specific key on an object.
@elasticmachine
Copy link
Contributor

💔 Build Failed

toddself added a commit that referenced this pull request May 24, 2019
* [feat] add reach-like functionality to object

In #36804 we need to validate a `Partial<HttpConfig>` object which may
or may not have all the required keys attached to it.  This is the type
of use-case for `Joi.reach`, so we should expose a reach-like method on
the object validation class so that you can return the validator for a
specific key on an object.

* [fix] change to validateKey

* [fix] use throw error matcher

* [fix] use same interface for validate

* [fix] change test name]
@toddself toddself force-pushed the multiple-http-service branch from 904be16 to c2489d2 Compare May 24, 2019 23:18
@toddself toddself force-pushed the multiple-http-service branch from 279d414 to 6bb33b4 Compare May 29, 2019 14:39
@elasticmachine
Copy link
Contributor

💔 Build Failed

Copy link
Contributor

@joshdover joshdover left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few small things, good to merge once resolved.

src/core/server/http/http_server.ts Outdated Show resolved Hide resolved
src/core/server/http/http_server.ts Outdated Show resolved Hide resolved
src/core/server/http/http_service.ts Outdated Show resolved Hide resolved
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@toddself toddself merged commit 461a6c0 into elastic:master May 29, 2019
@toddself toddself deleted the multiple-http-service branch May 29, 2019 20:01
toddself added a commit to toddself/kibana that referenced this pull request May 29, 2019
* [feat] create additional http servers

allow for additional http servers to be created, tracked and returned

* respond to pr feedback

* tweak test

* update documentation

* destructure port, remove unnecessary imports

* [fix] export correct type

* [feat] expose createNewServer to plugins

* [fix] respond to pr feedback

* todo: add schema validation & integration test

* use reach

* [fix] use validateKey to validate partial

* [fix] change config shadowing

* check kibana port & prevent shadowing

* centralize start/stop for servers, add integration test

* remove unnecessary property

* never forget your await

* remove option to pass config into start

* fix pr feedback

* fix documentation

* fix test failures
isListening: () => this.httpServer.isListening(),
isListening: (port = 0) => {
const server = this.secondaryServers.get(port);
if (server) return server.isListening();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we differentiate between primary server and secondary? that creates additional logic

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i originally didn't differentiate them but thought we might want to since we might want ot know what the primary instance is (than again we could find the port for primary and get it that way).

anyway i created an issue for it #37695.

jkakavas pushed a commit to jkakavas/kibana that referenced this pull request May 30, 2019
* [feat] create additional http servers

allow for additional http servers to be created, tracked and returned

* respond to pr feedback

* tweak test

* update documentation

* destructure port, remove unnecessary imports

* [fix] export correct type

* [feat] expose createNewServer to plugins

* [fix] respond to pr feedback

* todo: add schema validation & integration test

* use reach

* [fix] use validateKey to validate partial

* [fix] change config shadowing

* check kibana port & prevent shadowing

* centralize start/stop for servers, add integration test

* remove unnecessary property

* never forget your await

* remove option to pass config into start

* fix pr feedback

* fix documentation

* fix test failures
toddself added a commit to toddself/kibana that referenced this pull request May 30, 2019
* [feat] create additional http servers

allow for additional http servers to be created, tracked and returned

* respond to pr feedback

* tweak test

* update documentation

* destructure port, remove unnecessary imports

* [fix] export correct type

* [feat] expose createNewServer to plugins

* [fix] respond to pr feedback

* todo: add schema validation & integration test

* use reach

* [fix] use validateKey to validate partial

* [fix] change config shadowing

* check kibana port & prevent shadowing

* centralize start/stop for servers, add integration test

* remove unnecessary property

* never forget your await

* remove option to pass config into start

* fix pr feedback

* fix documentation

* fix test failures
toddself added a commit that referenced this pull request May 31, 2019
* [feat] create additional http servers (#36804)

* [feat] create additional http servers

allow for additional http servers to be created, tracked and returned

* respond to pr feedback

* tweak test

* update documentation

* destructure port, remove unnecessary imports

* [fix] export correct type

* [feat] expose createNewServer to plugins

* [fix] respond to pr feedback

* todo: add schema validation & integration test

* use reach

* [fix] use validateKey to validate partial

* [fix] change config shadowing

* check kibana port & prevent shadowing

* centralize start/stop for servers, add integration test

* remove unnecessary property

* never forget your await

* remove option to pass config into start

* fix pr feedback

* fix documentation

* fix test failures

* [fix] failing negation on merge
@joshdover joshdover added the release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. label Jul 9, 2019
@elasticmachine
Copy link
Contributor

💔 Build Failed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. reverted Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

core/http needs to allow for additional http services to be spun up
5 participants