cleanup configuration options #61
Conversation
|
@sat939 There is still configuration accessed here: https://github.com/LabShare/services/blob/master/lib/server-utils.js#L35. Could you update the parameters so that services can pass in https configuration from the constructor? |
sample-config.json
Outdated
| "ServicePath": "", | ||
| "HTTPS": { | ||
| "PrivateKey": "", | ||
| "Certificate": "" | ||
| }, | ||
| "loadServices" : true, | ||
| "pattern" : "", |
There was a problem hiding this comment.
@sat939 Can you remove pattern, main, and directories from the sample config please?
lib/services.js
Outdated
| }); | ||
|
|
||
| this.server = serverUtils.createServer(this._app, this._options.logger); | ||
| this._apiLoader = new Loader(this._app, this._options); | ||
| this._socketLoader = new SocketIOLoader(this.server, this._options); | ||
|
|
||
| console.log(this._options); |
sample-config.json
Outdated
| "pattern" : "", | ||
| "main" : "", | ||
| "directories" : [], | ||
| "morgan" : { |
There was a problem hiding this comment.
@sat939 Since defaultsDeep doesn't override empty values, I would prefer if the 'morgan' entry was an empty object. That will ensure the default values are used even when the sample config is used as a template.
|
@sat939 Can you update https://github.com/LabShare/services/blob/master/docs/socket-apis.md too since the configuration for establishing P2P socket connections changed? |
lib/services.js
Outdated
| @@ -37,7 +37,9 @@ class Services { | |||
| this._isProduction = this._app.get('env') === 'production'; | |||
| this._options = _.defaultsDeep(options, { | |||
| logger, | |||
| connections: [], | |||
| socket:{ | |||
There was a problem hiding this comment.
@sat939 Changing the option here will require the option handling in https://github.com/LabShare/services/blob/master/lib/api/socket-io-loader.js#L56 to be updated too. It will also require an update to the tests and to the tests in https://github.com/LabShare/storage/blob/master/test/lib/unit/proxy/storage_spec.js.
resolves #52