HPCC-12426 Add WUQueryDetails option to list applicable WsEcl Addresses #6617
Conversation
Signed-off-by: Anthony Fishbeck <anthony.fishbeck@lexisnexis.com>
|
Jira updated |
|
@GordonSmith Please review |
|
Looks good. |
|
@jakesmith Please review for dali impact |
| const char *netAddr = instance.queryProp("@netAddress"); | ||
| if (!netAddr || !*netAddr) | ||
| continue; | ||
| if (streq(netAddr, ".") && daliAddress.length()) |
There was a problem hiding this comment.
Looks odd - if netAddr = "." - then assume EspProcess/Instance is on dali node?
I'm not clear why you need to special case "." here, but if it is in the environment, then it surely means everything i, includes this esp node.
So you should probably use SocketEndpoint to resolve ".".
There was a problem hiding this comment.
I don't like that "." is used for a netAddress in the environment at all. Since the ESP running this code may be one with an explicitly configured address, while others have ".", I have to assume "." means the address of the dali. Weird but the safest way I think. If they move dali, they can't use "."... and then I'd be happier anyway.
There was a problem hiding this comment.
So you're thinking with environment with a mixture of "."'s and explicity netAddresses.. that would be weird..
I'm not sure what it means..
I think a comment at very least is needed, but really I think since "." means my local IP, it it SocketEndpoint(".") does not match your local IP - then I'd be temped to error with a mis-configuration issue.
There was a problem hiding this comment.
Ok I'll first think about looking up the computer reference rather than using the netAddress.
|
I don't have any efficiency concerns, as this is interacting with a cached copy of the environment. Not something new to this code, but really retrieving information from the Environment should be better supported by the IConstEnvironment interface. @afishbeck - a few minor comments |
Signed-off-by: Anthony Fishbeck <anthony.fishbeck@lexisnexis.com>
|
@jakesmith updated based on review. |
|
Looks good to merge to me. |
HPCC-12426 Add WUQueryDetails option to list applicable WsEcl Addresses Reviewed-By: Jake Smith <jake.smith@lexisnexis.com> Reviewed-By: Gordon Smith <gordon.smith@lexisnexis.com> Reviewed-By: Richard Chapman <rchapman@hpccsystems.com>
Signed-off-by: Anthony Fishbeck anthony.fishbeck@lexisnexis.com