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/04/26 17:51:23 UTC

[GitHub] [couchdb] iilyak opened a new pull request #3529: Create couch lib

iilyak opened a new pull request #3529:
URL: https://github.com/apache/couchdb/pull/3529


   ## Overview
   
   Create `couch_lib` application to contain common functions. This library application is pure in a sense that it MUST not use any other modules from CouchDB to avoid circular dependencies.
   
   ## Testing recommendations
   
   ```
   make && make eunit apps=couch_views
   ```
   
   ## Related Issues or Pull Requests
   
   - This PR is a replacement for https://github.com/apache/couchdb/pull/3525
   
   ## Checklist
   
   - [x] Code is written and works correctly
   - [x] 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] nickva edited a comment on pull request #3529: Create couch lib

Posted by GitBox <gi...@apache.org>.
nickva edited a comment on pull request #3529:
URL: https://github.com/apache/couchdb/pull/3529#issuecomment-827044484


   Since there are only 3 or 4 functions, why not put them couch app?


-- 
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] iilyak edited a comment on pull request #3529: Create couch lib

Posted by GitBox <gi...@apache.org>.
iilyak edited a comment on pull request #3529:
URL: https://github.com/apache/couchdb/pull/3529#issuecomment-827132969


   > We'd also want keep some correspondence between 3.x and main for the parts which haven't changed, like say couch_key_tree (instead of moving it to couch_lib), so that if we find any bugs it would be nice if a cherry-pick would still apply there without too much difficulty.
   
   I propose `couch_lib` only for non CouchDB specific things. Think of it as extension for OTP. Erlang/OTP stdlib missing quite a lot of useful things and its API is very inconsistent. The `couch_lib`
   would contain pure functions only. 
   
   However I don't want to put myself in the corner so I think we can consider adding supervised processes there eventually. Things like erlang tracing helper could find home there. However we need to be careful so the things we add are not CouchDB specific. For example we shouldn't call `couch_log` or `config` from `couch_lib`.


-- 
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] iilyak edited a comment on pull request #3529: Create couch lib

Posted by GitBox <gi...@apache.org>.
iilyak edited a comment on pull request #3529:
URL: https://github.com/apache/couchdb/pull/3529#issuecomment-827109368


   > Since there are only 3 or 4 functions, why not put them couch app?
   
   @nickva Few reasons:
   1. I remember Paul mentioned a vision where `couch` app would eventually go away so we shouldn't add new functions to it. He didn't mention the details so I interpret his vision as we would remove all functionality from `couch` app and then probably rename `fabric` into `couch`.
   2. There are few applications which depend on `couch`. This creates circular dependency between applications. We have a very nasty instance of this problem between `config` and `couch_log`. It makes things harder to use/release/test.
   3. We need a place where we can put auxiliary functions useful from any application. Things like:
   - couch_util:to_hex
   - couch_util:integer_to_boolean
   - function to manipulate JSON (extract and modify)
   The fact that these are in `couch` was a deal breaker few times already. 


-- 
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] nickva commented on pull request #3529: Create couch lib

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


   Since there are only 3 or 4 functions, why not put in couch app?


-- 
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] nickva commented on pull request #3529: Create couch lib

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


   @iilyak that makes, good idea
   


-- 
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] nickva edited a comment on pull request #3529: Create couch lib

Posted by GitBox <gi...@apache.org>.
nickva edited a comment on pull request #3529:
URL: https://github.com/apache/couchdb/pull/3529#issuecomment-827133533


   @iilyak that makes sense, good idea
   


-- 
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] iilyak commented on pull request #3529: Create couch lib

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


   > We'd also want keep some correspondence between 3.x and main for the parts which haven't changed, like say couch_key_tree (instead of moving it to couch_lib), so that if we find any bugs it would be nice if a cherry-pick would still apply there without too much difficulty.
   
   I propose `couch_lib` only for non CouchDB specific things. Think of it as extension for OTP. Erlang/OTP stdlib missing quite a lot of useful things and its API is very inconsistent. The `couch_lib`
   would contain pure functions only. 
   
   However I don't want to put myself in the corner so I think we can consider adding supervised processes there eventually. Things like erlang tracing helper could find home there. However we need to be careful so the things we add are not CouchDB specific. For example we shouldn't call `couch_log` or `config` from couch_lib`.


-- 
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] nickva commented on pull request #3529: Create couch lib

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


   What would be useful, I think, is to have a `couch_fdb` app which would handle basic FDB functionality at the directory + layer level not at the CouchDB db handle level. Then, `couch_jobs`, `couch_expiring_cache` and `fabric2` would use it without circular dependencies between them. Currently `couch_jobs` depends on `fabric2_fdb`, and `fabric2` db expiration depends on `couch_jobs` in turn too, which is not great. But that's a different PR and discussion...
   
   `couch` app itself is almost a library app. It has admin password hashing, couch_drv, couch_js and some EPI functionality which is "active" but those are fairly low level and not too bad when it comes to circular dependencies.
   We'd also want keep some correspondence between 3.x and main for the parts which haven't changed, like say `couch_key_tree` (instead of moving it to couch_lib), so that if we find any bugs it would be nice if a cherry-pick would still apply there without too much difficulty. 


-- 
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] iilyak commented on pull request #3529: Create couch lib

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


   > > Since there are only 3 or 4 functions, why not put them couch app?
   > 
   > Few reasons:
   > 
   >     1. I remember Paul mentioned a vision where `couch` app would eventually go away so we shouldn't add new functions to it. ...
   
   Never mind I misremembered he was talking about `couch_mrview` (see https://github.com/apache/couchdb/pull/2870#discussion_r424656725).


-- 
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] iilyak merged pull request #3529: Create couch lib

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


   


-- 
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] iilyak commented on pull request #3529: Create couch lib

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


   > Since there are only 3 or 4 functions, why not put them couch app?
   
   Few reasons:
   1. I remember Paul mentioned a vision where `couch` app would eventually go away so we shouldn't add new functions to it. He didn't mention the details so I interpret his vision as we would remove all functionality from `couch` app and then probably rename `fabric` into `couch`.
   2. There are few applications which depend on `couch`. This creates circular dependency between applications. We have a very nasty instance of this problem between `config` and `couch_log`. It makes things harder to use/release/test.
   3. We need a place where we can put auxiliary functions useful from any application. Things like:
   - couch_util:to_hex
   - couch_util:integer_to_boolean
   - function to manipulate JSON (extract and modify)
   The fact that these are in `couch` was a deal breaker few times already. 


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