You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by "jcoglan (via GitHub)" <gi...@apache.org> on 2023/11/30 11:10:39 UTC
[PR] Look up search node name in config for weatherreport [couchdb]
jcoglan opened a new pull request, #4887:
URL: https://github.com/apache/couchdb/pull/4887
## Overview
Weatherreport is hard-coded to assume the search node is at 'clouseau@127.0.0.1' and that it is running. This results in a useless warning for users that do not have search enabled, and also means that it will fail to connect if Clouseau is running somewhere other than 127.0.0.1.
This replaces the hard-coded node name with a config lookup to `[dreyfus] name` and handles the case of it not being set.
## Testing recommendations
On `main`, running `weatherreport` against my dev cluster prints this warning about Clouseau not responding, regardless of what `[dreyfus] name` is set to:
```
$ ./weatherreport -c ../../dev/lib/node1/etc
['node1@127.0.0.1'] [warning] Local search node at 'clouseau@127.0.0.1' not responding: pang
```
With this change, the above warning is not printed unless `[dreyfus] name` has been set:
```
$ cdb '/_node/_local/_config/dreyfus/name' -X DELETE
"clouseau1@127.0.0.1"
$ ./weatherreport -c ../../dev/lib/node1/etc
(no warning)
$ cdb '/_node/_local/_config/dreyfus/name' -X PUT -d '"clouseau@example.com"'
""
$ ./weatherreport -c ../../dev/lib/node1/etc
['node1@127.0.0.1'] [warning] Local search node at 'clouseau@example.com' not responding: pang
```
## Checklist
- [ ] Code is written and works correctly
- [ ] Changes are covered by tests
- [ ] Any new configurable parameters are documented in `rel/overlay/etc/default.ini`
- [ ] Documentation changes were made in the `src/docs` folder
- [ ] Documentation changes were backported (separated PR) to affected branches
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Look up search node name in config for weatherreport [couchdb]
Posted by "pgj (via GitHub)" <gi...@apache.org>.
pgj commented on code in PR #4887:
URL: https://github.com/apache/couchdb/pull/4887#discussion_r1411467012
##########
src/weatherreport/src/weatherreport_check_search.erl:
##########
@@ -54,7 +69,11 @@ check(_Opts) ->
end.
-spec format(term()) -> {io:format(), [term()]}.
+format(clouseau_not_configured) ->
+ {"Clouseau service is not configured", []};
format({clouseau_ok, SearchNode}) ->
{"Local search node at ~w responding ok", [SearchNode]};
+format({clouseau_node, SearchNode}) ->
+ {"Search node name ~s is not recognised", [SearchNode]};
Review Comment:
"recognised" is the UK spelling. I think the US spelling ("recognized") is used in this project, but I may be wrong here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Look up search node name in config for weatherreport [couchdb]
Posted by "rnewson (via GitHub)" <gi...@apache.org>.
rnewson commented on code in PR #4887:
URL: https://github.com/apache/couchdb/pull/4887#discussion_r1410534058
##########
src/weatherreport/src/weatherreport_check_search.erl:
##########
@@ -44,16 +44,24 @@ valid() ->
-spec check(list()) -> [{atom(), term()}].
check(_Opts) ->
- SearchNode = 'clouseau@127.0.0.1',
- case net_adm:ping(SearchNode) of
- pong ->
- [{info, {clouseau_ok, SearchNode}}];
- Error ->
- % only warning since search is not enabled by default
- [{warning, {clouseau_error, SearchNode, Error}}]
+ SearchNodeStr = config:get("dreyfus", "name"),
+ case SearchNodeStr of
+ undefined ->
+ [{info, clouseau_not_configured}];
+ _ ->
+ SearchNode = list_to_atom(SearchNodeStr),
Review Comment:
needs to be `list_to_existing_atom`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Look up search node name in config for weatherreport [couchdb]
Posted by "janl (via GitHub)" <gi...@apache.org>.
janl merged PR #4887:
URL: https://github.com/apache/couchdb/pull/4887
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Look up search node name in config for weatherreport [couchdb]
Posted by "jcoglan (via GitHub)" <gi...@apache.org>.
jcoglan commented on code in PR #4887:
URL: https://github.com/apache/couchdb/pull/4887#discussion_r1410811694
##########
src/weatherreport/src/weatherreport_check_search.erl:
##########
@@ -44,16 +44,24 @@ valid() ->
-spec check(list()) -> [{atom(), term()}].
check(_Opts) ->
- SearchNode = 'clouseau@127.0.0.1',
- case net_adm:ping(SearchNode) of
- pong ->
- [{info, {clouseau_ok, SearchNode}}];
- Error ->
- % only warning since search is not enabled by default
- [{warning, {clouseau_error, SearchNode, Error}}]
+ SearchNodeStr = config:get("dreyfus", "name"),
+ case SearchNodeStr of
+ undefined ->
+ [{info, clouseau_not_configured}];
+ _ ->
+ SearchNode = list_to_atom(SearchNodeStr),
Review Comment:
I've pushed one more commit that implements this, is it what you had in mind?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org