Feat/multi query management #255
Conversation
|
The only other thing besides my comments above is that you might want to write and test query updates now, since you're already doing most of the work, vs later. |
|
Also, we would now need to make POST /query a privileged request, but keep the GETs unprivileged. We should probably address that in this PR also. |
adapters/ClientxServiceAdapter.mjs
Outdated
| import { logger } from '../lib/logger.mjs'; | ||
| import * as cmn from '../lib/common.mjs'; | ||
|
|
||
| class ClientxServiceAdapter { |
There was a problem hiding this comment.
Why does this class have such a generic name? We have multiple clients and services in our code base, and this name gives the reader no info about what the class actually acts upon. Let's call it ARSCallbackxQueryServiceAdapter.
There was a problem hiding this comment.
My thinking here was that is that the client could be generic, there is nothing specifically that ties it to a callback mechanism.
services/QueryService.mjs
Outdated
| import * as cmn from '../lib/common.mjs'; | ||
|
|
||
| class QueryService { | ||
| constructor(queryStore, clientAdapter, feAdapter) { |
There was a problem hiding this comment.
callbackAdapter or callbackPayloadAdapter would be a better param name than clientAdapter.
There was a problem hiding this comment.
My thinking here was that is that the client could be generic, there is nothing specifically that ties it to a callback mechanism.
* Changed ClientxServiceAdapter to ARSCallbackxQueryServiceAdapter * Remove v2 API path
No description provided.