-
Notifications
You must be signed in to change notification settings - Fork 304
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
HPCC-31196 Add "disconnect", "unlock", etc. to WsDali #18342
Conversation
Add "disconnect", "unlock", "settracetransactions", "settraceslowtransactions", "cleartracetransactions", "save" (from dalidiag) to the WsDali service. Signed-off-by: wangkx <[email protected]>
https://track.hpccsystems.com/browse/HPCC-31196 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wangkx - looks good, just a couple of minor comments.
esp/scm/ws_dali.ecm
Outdated
@@ -151,8 +151,36 @@ ESPrequest [nil_remove] GetSDSSubscribersRequest | |||
{ | |||
}; | |||
|
|||
ESPrequest [nil_remove] DisconnectClientConnectionRequest | |||
{ | |||
string URL; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a url is misleading. It's a host:port. Endpoint would be better.
|
||
const char* url = req.getURL(); | ||
if (isEmptyString(url)) | ||
throw makeStringException(ECLWATCH_INVALID_INPUT, "URL not specified."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see prev. comment re. "endpoint"
throw makeStringException(ECLWATCH_INVALID_INPUT, "ConnectionID not specified."); | ||
|
||
MemoryBuffer mb; | ||
mb.append("unlock").append(strtoll(connectionIdHex, nullptr, 16)).append(req.getClose()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to check this, because it's not clear from the dalidiag usage (not a new issue) that it expects a hex number.
I think worth updating the daliadmin usage text at the same time to note it expect the connectionID to be hex.
1. Inside the DisconnectClientConnectionRequest, rename the URL to Endpoint. 2. Update the usage text for dalidiag -unlock. Signed-off-by: wangkx <[email protected]>
@jakesmith please review my changes and let me know if I miss any. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wangkx - looks fine. Please squash.
Add "disconnect", "unlock", "settracetransactions", "settraceslowtransactions", "cleartracetransactions", "save" (from dalidiag) to the WsDali service.
Type of change:
Checklist:
Smoketest:
Testing: