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/06/15 00:22:10 UTC

[GitHub] [couchdb] jaydoane opened a new pull request #3629: Fix custodian default system dbs

jaydoane opened a new pull request #3629:
URL: https://github.com/apache/couchdb/pull/3629


   These system db defaults were left unchanged when this code was
   imported from Cloudant. This changes them to CouchDB defaults.
   
   ## Checklist
   
   - [x] Code is written and works correctly
   - [ ] Changes are covered by tests
   - [ ] 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] jaydoane commented on a change in pull request #3629: Fix custodian default system dbs

Posted by GitBox <gi...@apache.org>.
jaydoane commented on a change in pull request #3629:
URL: https://github.com/apache/couchdb/pull/3629#discussion_r651371234



##########
File path: src/custodian/src/custodian_db_checker.erl
##########
@@ -132,7 +132,7 @@ get_dbs() ->
 
 
 get_users_db() ->
-    UsersDb = config:get("couch_httpd_auth", "authentication_db", "users"),
+    UsersDb = config:get("couch_httpd_auth", "authentication_db", "_users"),

Review comment:
       Fixed up with exported `dbname/0`

##########
File path: src/custodian/src/custodian_db_checker.erl
##########
@@ -132,7 +132,7 @@ get_dbs() ->
 
 
 get_users_db() ->
-    UsersDb = config:get("couch_httpd_auth", "authentication_db", "users"),
+    UsersDb = config:get("couch_httpd_auth", "authentication_db", "_users"),

Review comment:
       That particular function is not exported. If you prefer, I can also export `dbname/0` from chttpd_auth_cache and then use it.




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] jaydoane commented on a change in pull request #3629: Fix custodian default system dbs

Posted by GitBox <gi...@apache.org>.
jaydoane commented on a change in pull request #3629:
URL: https://github.com/apache/couchdb/pull/3629#discussion_r651367033



##########
File path: src/custodian/src/custodian_db_checker.erl
##########
@@ -132,7 +132,7 @@ get_dbs() ->
 
 
 get_users_db() ->
-    UsersDb = config:get("couch_httpd_auth", "authentication_db", "users"),
+    UsersDb = config:get("couch_httpd_auth", "authentication_db", "_users"),

Review comment:
       That particular function is not exported. If you prefer, I can also export `dbname/1` from chttpd_auth_cache and then use it.




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] jaydoane merged pull request #3629: Fix custodian default system dbs

Posted by GitBox <gi...@apache.org>.
jaydoane merged pull request #3629:
URL: https://github.com/apache/couchdb/pull/3629


   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] chewbranca commented on a change in pull request #3629: Fix custodian default system dbs

Posted by GitBox <gi...@apache.org>.
chewbranca commented on a change in pull request #3629:
URL: https://github.com/apache/couchdb/pull/3629#discussion_r651362440



##########
File path: src/custodian/src/custodian_db_checker.erl
##########
@@ -132,7 +132,7 @@ get_dbs() ->
 
 
 get_users_db() ->
-    UsersDb = config:get("couch_httpd_auth", "authentication_db", "users"),
+    UsersDb = config:get("couch_httpd_auth", "authentication_db", "_users"),

Review comment:
       Any reason to not use: https://github.com/apache/couchdb/blob/3.x/src/chttpd/src/chttpd_auth_cache.erl#L188-L189




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] chewbranca commented on a change in pull request #3629: Fix custodian default system dbs

Posted by GitBox <gi...@apache.org>.
chewbranca commented on a change in pull request #3629:
URL: https://github.com/apache/couchdb/pull/3629#discussion_r651362005



##########
File path: src/custodian/src/custodian_util.erl
##########
@@ -43,7 +43,7 @@ report() ->
     fold_dbs([], Fun).
 
 ensure_dbs_exists() ->
-    DbName = config:get("mem3", "shards_db", "dbs"),
+    DbName = config:get("mem3", "shards_db", "_dbs"),

Review comment:
       Any reason to not use: https://github.com/apache/couchdb/blob/3.x/src/mem3/src/mem3_sync.erl#L315-L316




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] jaydoane commented on a change in pull request #3629: Fix custodian default system dbs

Posted by GitBox <gi...@apache.org>.
jaydoane commented on a change in pull request #3629:
URL: https://github.com/apache/couchdb/pull/3629#discussion_r651365342



##########
File path: src/custodian/src/custodian_util.erl
##########
@@ -43,7 +43,7 @@ report() ->
     fold_dbs([], Fun).
 
 ensure_dbs_exists() ->
-    DbName = config:get("mem3", "shards_db", "dbs"),
+    DbName = config:get("mem3", "shards_db", "_dbs"),

Review comment:
       Other than laziness, the only reason is that I was recently chastised by @rnewson for using a function in a different module that was not in an `_util` module. However, I do prefer keeping things DRY, so I've changed it!




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] rnewson commented on pull request #3629: Fix custodian default system dbs

Posted by GitBox <gi...@apache.org>.
rnewson commented on pull request #3629:
URL: https://github.com/apache/couchdb/pull/3629#issuecomment-874790032


   I was speaking of the convention of only calling the "top level" module of an application, in order to allow changes within an application with minimal risk of breaking couchdb as a whole. so if we need something from mem3 from other places, add a function in mem3.erl that delegates to the internal mem3_sync module. high cohesion / low coupling.


-- 
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