I15

[balmas]

this fixes #15 by pulling the lexicon configuration from the alpheios-config-api and passing it to the lexicons client adapter.

See also alpheios-project/alpheios-config-api#5

For now, I am using the config data supplied by the remote config api as an override to that which is built into the package. So this means that if the remote config api is down or unavailable, we would still be able to get short definitions, albeit they might not be the latest versions.

We still have a duplication of the list of available lexicons in the default settings config files, which aren't retrieved from the remote api yet.

Ultimately I would to remove all duplications of configuration but I think it's safer to move in small steps towards that. The immediate need for this incremental is to be able to deploy updated short definition files without breaking the prior release (the updated short defs files don't really break in the prior release, but they have some html tags that aren't accounted for in the prior release's css properly)

[balmas]

I have a suggestion - may be it is better to create a specific adapter for getting configuration?

This way it would support the common features:

• support windos.fetch and axios for the cases when we don't have windows object
• support handling errors
• support checking input parameters
• and also we would be able to use fixtures for it inside tests

you mean an adapter for the remote config service?

[irina060981]

you mean an adapter for the remote config service?

Yes. I believe that we should divide features between packages.

ClientAdapters is responsible for all remote requests. It has a common workflow for all fetching data (with the fallback to axios fetch and handling with errors). We have specific tests for that.

This method is the only one that does direct fetch to the remote resource from components package.

  async requestAppOptions () {
    const configUrl = `${this._configServiceUrl}?clientId=${encodeURIComponent(this._clientId)}&appName=${encodeURIComponent(this._appName)}` +
      `&appVersion=${encodeURIComponent(this._appVersion)}&buildBranch=${encodeURIComponent(this._branch)}` +
      `&buildNumber=${encodeURIComponent(this._buildNumber)}`
    const request = new Request(configUrl)
    return fetch(request)
  }

And also inside ClientAdapters it could have some default local config - if remote source would fail. And it would make code more consistant.

[balmas]

you mean an adapter for the remote config service?

Yes. I believe that we should divide features between packages.

ClientAdapters is responsible for all remote requests. It has a common workflow for all fetching data (with the fallback to axios fetch and handling with errors). We have specific tests for that.

This method is the only one that does direct fetch to the remote resource from components package.

  async requestAppOptions () {
    const configUrl = `${this._configServiceUrl}?clientId=${encodeURIComponent(this._clientId)}&appName=${encodeURIComponent(this._appName)}` +
      `&appVersion=${encodeURIComponent(this._appVersion)}&buildBranch=${encodeURIComponent(this._branch)}` +
      `&buildNumber=${encodeURIComponent(this._buildNumber)}`
    const request = new Request(configUrl)
    return fetch(request)
  }

And also inside ClientAdapters it could have some default local config - if remote source would fail. And it would make code more consistant.

I guess I see this a little differently. The role of the ClientAdapters is to adapt external resources to our internal models. So, while the base adapter does provide a common encapsulation of the XHR request, I don't think that means that all XHR requests need to go through the Client Adapters.

The role of the configuration service is to provide application-specific properties at runtime that might change separately from releases of the applications.

I do agree, however, we could have a better way of making this data available to the client adapters than passing it in the individual adapter method calls as a parameter. This should be something we consider for the next major release. For the purposes of this PR, which is for an incremental release, I'd like to limit the scope of the changes.