You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by GitBox <gi...@apache.org> on 2021/09/27 17:21:15 UTC
[GitHub] [couchdb] jiahuili430 opened a new pull request #3767: Add search to list of features in welcome response
jiahuili430 opened a new pull request #3767:
URL: https://github.com/apache/couchdb/pull/3767
## Overview
Return `search` in the `features` list from the welcome endpoint. Something like:
```
curl http://adm:pass@127.0.0.1:15984 | jq '.'
"couchdb": "Welcome",
"version": "3.1.1-9f08191",
"git_sha": "9f08191",
"uuid": "fake_uuid_for_dev",
"features": [
"access-ready",
"partitioned",
"pluggable-storage-engines",
"reshard",
"scheduler",
"search"
],
"vendor": {
"name": "The Apache Software Foundation"
}
}
```
To make it configurable, a configuration option has been added.
```
[dreyfus]
enable = false
```
## Testing recommendations
## Related Issues or Pull Requests
Issue [#70](https://github.ibm.com/cloudant/dbcore/issues/70)
## Checklist
- [x] Code is written and works correctly
- [ ] Changes are covered by tests
- [x] Any new configurable parameters are documented in `rel/overlay/etc/default.ini`
- [ ] A PR for documentation changes has been made in https://github.com/apache/couchdb-documentation
--
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
[GitHub] [couchdb] nickva commented on a change in pull request #3767: Add search to list of features in welcome response
Posted by GitBox <gi...@apache.org>.
nickva commented on a change in pull request #3767:
URL: https://github.com/apache/couchdb/pull/3767#discussion_r720418837
##########
File path: src/chttpd/src/chttpd_misc.erl
##########
@@ -60,7 +60,13 @@ handle_welcome_req(Req, _) ->
send_method_not_allowed(Req, "GET,HEAD").
get_features() ->
- case clouseau_rpc:connected() of
+ Clouseau =
+ ['clouseau1@127.0.0.1', 'clouseau2@127.0.0.1', 'clouseau3@127.0.0.1'],
+ Connected = lists:foldl(
+ fun (X, Y) ->
+ clouseau_rpc:connected(X) orelse Y
+ end, false, Clouseau),
+ case Connected orelse clouseau_rpc:connected() of
Review comment:
It doesn't seem right on first sight that we hard code 3 node names in the features list check.
I'd think if there is a clouseau node configuration setting we should probably read that.
Another idea would be to encapsulate this in the dreyfus app somewhere. Have some `dreyfus:feature_available/0 -> true|false` function perhaps, so the knowledge about the exact node names is hidden away from chttpd_misc module.
The logic would be something like:
* If dreyfus is enabled AND
* We can connect to clouseau we show feature as enabled.
I think we could probably add some caching and assume that if we could connect to it at least once in the past few minutes should be able to return true, otherwise return false. Since / endpoint ends up being hit unauthenticated we don't want to expose ourself to a DoS attack some some repeatedly flooding the dist connection to clouseau on every `GET /` request.
My knowledge of search is limited, I think we may need some help from the @rnewson or @tonysun83 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
[GitHub] [couchdb] nickva commented on pull request #3767: Add search to list of features in welcome response
Posted by GitBox <gi...@apache.org>.
nickva commented on pull request #3767:
URL: https://github.com/apache/couchdb/pull/3767#issuecomment-928110718
Agree with @wohali. We should only return "search" in the features list if users can successfully use the feature. If `search` appears as a feature but search doesn't actually work, it look be quite confusing.
--
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
[GitHub] [couchdb] jiahuili430 commented on pull request #3767: Add search to list of features in welcome response
Posted by GitBox <gi...@apache.org>.
jiahuili430 commented on pull request #3767:
URL: https://github.com/apache/couchdb/pull/3767#issuecomment-934029882
Since we have [chttpd_misc:get_features()](https://github.com/apache/couchdb/blob/3.x/src/chttpd/src/chttpd_misc.erl#L62-L68) to add `search` to the features list, so I will close this PR.
--
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
[GitHub] [couchdb] nickva edited a comment on pull request #3767: Add search to list of features in welcome response
Posted by GitBox <gi...@apache.org>.
nickva edited a comment on pull request #3767:
URL: https://github.com/apache/couchdb/pull/3767#issuecomment-928110718
--
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
[GitHub] [couchdb] jiahuili430 closed pull request #3767: Add search to list of features in welcome response
Posted by GitBox <gi...@apache.org>.
jiahuili430 closed pull request #3767:
URL: https://github.com/apache/couchdb/pull/3767
--
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
[GitHub] [couchdb] wohali commented on pull request #3767: Add search to list of features in welcome response
Posted by GitBox <gi...@apache.org>.
wohali commented on pull request #3767:
URL: https://github.com/apache/couchdb/pull/3767#issuecomment-928092100
This needs an automated test. As we cannot run search in our automated test suite, the test should validate that, after loading dreyfus with no clouseau connected, and the default `search = true`, `search` is *not* returned by `GET /`.
--
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
[GitHub] [couchdb] nickva edited a comment on pull request #3767: Add search to list of features in welcome response
Posted by GitBox <gi...@apache.org>.
nickva edited a comment on pull request #3767:
URL: https://github.com/apache/couchdb/pull/3767#issuecomment-928110718
--
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
[GitHub] [couchdb] wohali commented on pull request #3767: Add search to list of features in welcome response
Posted by GitBox <gi...@apache.org>.
wohali commented on pull request #3767:
URL: https://github.com/apache/couchdb/pull/3767#issuecomment-928092100
This needs an automated test. As we cannot run search in our automated test suite, the test should validate that, after loading dreyfus with no clouseau connected, and the default `search = true`, `search` is *not* returned by `GET /`.
--
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
[GitHub] [couchdb] nickva commented on pull request #3767: Add search to list of features in welcome response
Posted by GitBox <gi...@apache.org>.
nickva commented on pull request #3767:
URL: https://github.com/apache/couchdb/pull/3767#issuecomment-928110718
Agree with @wohali. We should only return "search" in the features list if users can successfully use the feature. If `search` appears as a feature but search doesn't actually work, it look be quite confusing.
--
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