Fixes mapping issue when using :sysinfo with HA #599
Conversation
|
Could you add a test that shows that this error is gone, or is this component hard to run enzyme-compat tests on? |
oskarhane
changed the title
|
This is will require some refactoring I was trying to defer until later because on |
pe4cey
force-pushed
the
3.0-ha-sysinfo-fix
branch
from
ed45a1a to
002059e
Compare
pe4cey
force-pushed
the
3.0-ha-sysinfo-fix
branch
from
002059e to
913631a
Compare
|
@oskarhane added both component tests and tests around the extracted out logic to support this fix |
| canListQueries () { | ||
| return this.props.availableProcedures.includes('dbms.listQueries') | ||
| } | ||
|
|
||
| getRunningQueries (suppressQuerySuccessMessage = false) { | ||
| this.props.bus.self( | ||
| (this.isCC()) ? CLUSTER_CYPHER_REQUEST : CYPHER_REQUEST, | ||
| (this.props.isACausalCluster) ? CLUSTER_CYPHER_REQUEST : CYPHER_REQUEST, |
There was a problem hiding this comment.
How does this get passed to the :queries frame?
There was a problem hiding this comment.
Also, you'd need to change the line in killQueries below as well, where a check for this.isCC() is made.
There was a problem hiding this comment.
This change was by mistake. I didn't plan on changing anything outside of things that affect sysinfo.
| } | ||
|
|
||
| clusterResponseHandler () { | ||
| return (res) => { |
There was a problem hiding this comment.
Why return function rather than just handle the response directly?
It looks a bit unusual.
| } | ||
| } | ||
| responseHandler () { | ||
| return (res) => { |
There was a problem hiding this comment.
Same as in clusterResponseHandler.
It looks a bit unusual.
|
@oskarhane updated |
|
Very nice changes, lgtm. |
Fixes #586