You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@couchdb.apache.org by lemenkov <gi...@git.apache.org> on 2014/07/04 05:30:55 UTC

[GitHub] couchdb pull request: Allow passing directories as the config sour...

GitHub user lemenkov opened a pull request:

    https://github.com/apache/couchdb/pull/257

    Allow passing directories as the config sources

    We should allow passing the entire directories as the config sources. In
    this case CouchDB will traverse the given directories and load every
    *.ini file found.
    
    E.g.
    
    "... -couch_ini /etc/couchdb/default.d/ /etc/couchdb/local.d/ /path/to/extra/couchdb/default.ini /path/to/extra/couchdb/local.ini ..."
    
    Signed-off-by: Peter Lemenkov <le...@gmail.com>

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/lemenkov/couchdb ini-dirs-to-merge

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/couchdb/pull/257.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #257
    
----
commit adcd4c59f9fcd0469396658be837862dcda4f8b6
Author: Peter Lemenkov <le...@gmail.com>
Date:   2014-07-03T11:45:30Z

    Allow passing directories as the config sources
    
    We should allow passing the entire directories as the config sources. In
    this case CouchDB will traverse the given directories and load every
    *.ini file found.
    
    E.g.
    
    "... -couch_ini /etc/couchdb/default.d/ /etc/couchdb/local.d/ /path/to/extra/couchdb/default.ini /path/to/extra/couchdb/local.ini ..."
    
    Signed-off-by: Peter Lemenkov <le...@gmail.com>

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb pull request: Allow passing directories as the config sour...

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on the pull request:

    https://github.com/apache/couchdb/pull/257#issuecomment-48130098
  
    my thoughts captured here: https://github.com/rnewson/couchdb/compare/erlang-expand?expand=1


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb issue #257: Allow passing directories as the config sources

Posted by lemenkov <gi...@git.apache.org>.
Github user lemenkov commented on the issue:

    https://github.com/apache/couchdb/pull/257
  
    This PR was reworked, and applied upstream as apache/couchdb-config#9. Time to close this one.
    
    Thanks everyone!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb pull request: Allow passing directories as the config sour...

Posted by lemenkov <gi...@git.apache.org>.
Github user lemenkov commented on the pull request:

    https://github.com/apache/couchdb/pull/257#issuecomment-48009133
  
    @kxepal quite the contrary - it wil be handled correctly in any way. However I agree - that will be a source of confusion for further CouchDB code divers.
    



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb pull request: Allow passing directories as the config sour...

Posted by davisp <gi...@git.apache.org>.
Github user davisp commented on the pull request:

    https://github.com/apache/couchdb/pull/257#issuecomment-85251274
  
    Also, this should be ported to the couchdb-config app since this won't merge to master. Should be a trivial move though.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb pull request: Allow passing directories as the config sour...

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on the pull request:

    https://github.com/apache/couchdb/pull/257#issuecomment-48023013
  
    I'm confused. couchdb already loads all .ini files in the default.d and local.d directories. What is this adding?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb pull request: Allow passing directories as the config sour...

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on a diff in the pull request:

    https://github.com/apache/couchdb/pull/257#discussion_r14552437
  
    --- Diff: src/couchdb/couch_app.erl ---
    @@ -37,7 +38,17 @@ get_ini_files(Default) ->
         {ok, [[]]} ->
             Default;
         {ok, [Values]} ->
    -        Values
    +        lists:foldl(
    +		fun (V, AccIn) ->
    +			  case file:read_file_info(V) of
    +				  {ok, #file_info{type = regular}} -> AccIn ++ [V];
    --- End diff --
    
    should be [V | AccIn]


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb pull request: Allow passing directories as the config sour...

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:

    https://github.com/apache/couchdb/pull/257#discussion_r14574278
  
    --- Diff: src/couchdb/couch_app.erl ---
    @@ -37,7 +38,17 @@ get_ini_files(Default) ->
         {ok, [[]]} ->
             Default;
         {ok, [Values]} ->
    -        Values
    +        lists:foldl(
    +		fun (V, AccIn) ->
    +			  case file:read_file_info(V) of
    +				  {ok, #file_info{type = regular}} -> AccIn ++ [V];
    --- End diff --
    
    Order matters since otherwise we'll write all the changes to the first config in chain and it is default.ini.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb pull request: Allow passing directories as the config sour...

Posted by davisp <gi...@git.apache.org>.
Github user davisp commented on a diff in the pull request:

    https://github.com/apache/couchdb/pull/257#discussion_r26992836
  
    --- Diff: src/couchdb/couch_app.erl ---
    @@ -37,7 +38,17 @@ get_ini_files(Default) ->
         {ok, [[]]} ->
             Default;
         {ok, [Values]} ->
    -        Values
    +        lists:foldl(
    +		fun (V, AccIn) ->
    +			  case file:read_file_info(V) of
    +				  {ok, #file_info{type = regular}} -> AccIn ++ [V];
    --- End diff --
    
    Alternatively, use lists:foldr/3 and just do [V | AccIn] without the reverse.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb pull request: Allow passing directories as the config sour...

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on the pull request:

    https://github.com/apache/couchdb/pull/257#issuecomment-48023111
  
    finally, this area of code changes quite a bit after the 1843-feature-bigcouch merge.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb pull request: Allow passing directories as the config sour...

Posted by davisp <gi...@git.apache.org>.
Github user davisp commented on a diff in the pull request:

    https://github.com/apache/couchdb/pull/257#discussion_r26992810
  
    --- Diff: src/couchdb/couch_app.erl ---
    @@ -37,7 +38,17 @@ get_ini_files(Default) ->
         {ok, [[]]} ->
             Default;
         {ok, [Values]} ->
    -        Values
    +        lists:foldl(
    +		fun (V, AccIn) ->
    +			  case file:read_file_info(V) of
    +				  {ok, #file_info{type = regular}} -> AccIn ++ [V];
    +				  {ok, #file_info{type = directory}} -> AccIn ++ filelib:wildcard(filename:join([V,"*.ini"]));
    --- End diff --
    
    +1 to adding a sort.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb pull request #257: Allow passing directories as the config sources

Posted by lemenkov <gi...@git.apache.org>.
Github user lemenkov closed the pull request at:

    https://github.com/apache/couchdb/pull/257


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb pull request: Allow passing directories as the config sour...

Posted by wtogami <gi...@git.apache.org>.
Github user wtogami commented on the pull request:

    https://github.com/apache/couchdb/pull/257#issuecomment-48142750
  
    I was partially wrong ...
    ```
    type=AVC msg=audit(1404711494.997:6093): avc:  denied  { write } for  pid=19794 comm="beam.smp" name="default.ini" dev="dm-0" ino=672839 scontext=system_u:system_r:couchdb_t:s0 tcontext=system_u:object_r:couchdb_conf_t:s0 tclass=file
    ```
    couchdb does try to write to the first ini during startup.
    ```
    type=AVC msg=audit(1404711578.067:6096): avc:  denied  { write } for  pid=19793 comm="beam.smp" name="local.ini" dev="dm-0" ino=677149 scontext=system_u:system_r:couchdb_t:s0 tcontext=system_u:object_r:couchdb_conf_t:s0 tclass=file
    ```
    When you try to save a configuration via REST it does write to the last ini, so @rnewson's patch actually does work.
    
    Any idea where/why it opens the first ini file for writing during startup?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb pull request: Allow passing directories as the config sour...

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:

    https://github.com/apache/couchdb/pull/257#issuecomment-48007335
  
    LGFM. +1. It was fun to consume php configs on start (:
    The only notice is indention: we prefer 4 spaces over tabs - this makes your changes looks a bit alien in module.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb pull request: Allow passing directories as the config sour...

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on a diff in the pull request:

    https://github.com/apache/couchdb/pull/257#discussion_r14577835
  
    --- Diff: src/couchdb/couch_app.erl ---
    @@ -37,7 +38,17 @@ get_ini_files(Default) ->
         {ok, [[]]} ->
             Default;
         {ok, [Values]} ->
    -        Values
    +        lists:foldl(
    +		fun (V, AccIn) ->
    +			  case file:read_file_info(V) of
    +				  {ok, #file_info{type = regular}} -> AccIn ++ [V];
    +				  {ok, #file_info{type = directory}} -> AccIn ++ filelib:wildcard(filename:join([V,"*.ini"]));
    --- End diff --
    
    As noted in IRC, the ordering of filelib:wildcard is not specified, so this is unpredictable.
    
    To fix this, let's have [V | AccIn] for the earlier clause, preserve this clause, but add a final lists:sort(Values) for the function's result.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb pull request: Allow passing directories as the config sour...

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on the pull request:

    https://github.com/apache/couchdb/pull/257#issuecomment-48031358
  
    moved to: https://git-wip-us.apache.org/repos/asf?p=couchdb-config.git;a=blob;f=src/config_app.erl;h=5c5515af36e92b243a6f4c9d76890f3abaa280fa;hb=master


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb pull request: Allow passing directories as the config sour...

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on a diff in the pull request:

    https://github.com/apache/couchdb/pull/257#discussion_r14552447
  
    --- Diff: src/couchdb/couch_app.erl ---
    @@ -37,7 +38,17 @@ get_ini_files(Default) ->
         {ok, [[]]} ->
             Default;
         {ok, [Values]} ->
    -        Values
    +        lists:foldl(
    +		fun (V, AccIn) ->
    +			  case file:read_file_info(V) of
    +				  {ok, #file_info{type = regular}} -> AccIn ++ [V];
    --- End diff --
    
    (reverse at the end if the order mattered)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb pull request: Allow passing directories as the config sour...

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on a diff in the pull request:

    https://github.com/apache/couchdb/pull/257#discussion_r14574513
  
    --- Diff: src/couchdb/couch_app.erl ---
    @@ -37,7 +38,17 @@ get_ini_files(Default) ->
         {ok, [[]]} ->
             Default;
         {ok, [Values]} ->
    -        Values
    +        lists:foldl(
    +		fun (V, AccIn) ->
    +			  case file:read_file_info(V) of
    +				  {ok, #file_info{type = regular}} -> AccIn ++ [V];
    --- End diff --
    
    sure, just saying it's conventional to prepend then lists:reverse at the end.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb pull request: Allow passing directories as the config sour...

Posted by wtogami <gi...@git.apache.org>.
Github user wtogami commented on the pull request:

    https://github.com/apache/couchdb/pull/257#issuecomment-48139622
  
    @rnewson 
    Somehow both your and Peter's patch result in default.ini being opened for write.
    
    ```
    diff --git a/src/couchdb/couch_app.erl b/src/couchdb/couch_app.erl
    index d6d8c0c..9e5d7b5 100644
    --- a/src/couchdb/couch_app.erl
    +++ b/src/couchdb/couch_app.erl
    @@ -38,7 +38,8 @@ get_ini_files(Default) ->
         {ok, [[]]} ->
             Default;
         {ok, [Values]} ->
    -        lists:flatmap(fun(V) ->
    +        io:fwrite("INIT CHAIN: ~p~n", [Values]),
    +        R = lists:flatmap(fun(V) ->
                 case file:read_file_info(V) of
     		{ok, #file_info{type = regular}} ->
     		    [V];
    @@ -47,7 +48,9 @@ get_ini_files(Default) ->
     		{error, enoent} ->
     		    []
     	    end
    -	end, Values)
    +	end, Values),
    +	io:fwrite("RESULT CHAIN: ~p~n", [R]),
    +	R
         end.
     
     start_apps([]) ->
    ```
    The above patch applied on top of your patch confirms that default.ini is first.
    ```
    RESULT CHAIN: ["/etc/couchdb/default.ini","/etc/couchdb/default.d/1.ini",
    "/etc/couchdb/default.d/2.ini","/etc/couchdb/default.d/3.ini",
    "/etc/couchdb/local.d/1.ini","/etc/couchdb/local.d/2.ini",
    "/etc/couchdb/local.d/3.ini","/etc/couchdb/local.ini"]
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb pull request: Allow passing directories as the config sour...

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:

    https://github.com/apache/couchdb/pull/257#issuecomment-48007579
  
    Ah, I was a bit in rush:  if directory is specified with trailing slash it will not be handled correctly: the `couchdb -A /etc/couchdb/local.d/` call will look for `/etc/couchdb/local.d//*.ini` files and obliviously will find nothing. And since there would be no error this could make someone life a bit harder.
    
    Better replace: `filelib:wildcard(V ++ "/*.ini")`
    with: `filelib:wildcard(filename:join([V,"/*.ini"]));
    



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---