Mailing-List: contact dev-help@couchdb.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@couchdb.apache.org
Received-SPF: pass (nike.apache.org: domain of randall.leeds@gmail.com
 designates 209.85.160.52 as permitted sender)
MIME-Version: 1.0
In-Reply-To: 
 <CAL3q7H4gVHJUbdEMiMQFCXXpwTJuqJUXFaYJd9sS14uoKhmJFw@mail.gmail.com>
References: <20120319035640.1DC9F7B0D@tyr.zones.apache.org>
	<CAL3q7H4gVHJUbdEMiMQFCXXpwTJuqJUXFaYJd9sS14uoKhmJFw@mail.gmail.com>
Date: Mon, 19 Mar 2012 11:33:17 -0700
Message-ID: 
 <CAAL6JQj7eTZ4fzSD_=V+jGJpq2pqdJPCxdo-FBE-JdZioqwVSQ@mail.gmail.com>
Subject: Re: [3/3] git commit: remove unnecessary eaccess special casing
From: Randall Leeds <randall.leeds@gmail.com>
To: dev@couchdb.apache.org
Content-Type: multipart/alternative; boundary=047d7b33db50bc57b404bb9cca2e

--047d7b33db50bc57b404bb9cca2e
Content-Type: text/plain; charset=UTF-8

On Mon, Mar 19, 2012 at 04:48, Filipe David Manana <fdmanana@apache.org>wrote:

> Any special reason why couch_log and couch_file don't get the same
> treatment (properly formatted error message) as couch_config and
> couch_config_writer?
>

Hmm. I think maybe I should add it to couch_file because that makes a ton
of sense. At the time, I was thinking I would just let it bubble and it
would be caught above, as it certainly is in many cases.

For couch_log, how can we log anything? It felt futile to try to invoke log
calls when couch_log fails to start.


>
> Seems like a small (incidental) regression to me.
>
> thanks
>
> On Mon, Mar 19, 2012 at 3:56 AM,  <randall@apache.org> wrote:
> > remove unnecessary eaccess special casing
> >
> > It's better to let these errors bubble and/or not give them special
> > treatment when file:format_error/1 can do a better job of describing
> > the failure.
> >
> >
> > Project: http://git-wip-us.apache.org/repos/asf/couchdb/repo
> > Commit: http://git-wip-us.apache.org/repos/asf/couchdb/commit/48371335
> > Tree: http://git-wip-us.apache.org/repos/asf/couchdb/tree/48371335
> > Diff: http://git-wip-us.apache.org/repos/asf/couchdb/diff/48371335
> >
> > Branch: refs/heads/COUCHDB-1445
> > Commit: 483713352cb397510111798c4b076afad83f6c46
> > Parents: 29eac04
> > Author: Randall Leeds <randall@apache.org>
> > Authored: Sun Mar 18 20:19:27 2012 -0700
> > Committer: Randall Leeds <randall@apache.org>
> > Committed: Sun Mar 18 20:41:42 2012 -0700
> >
> > ----------------------------------------------------------------------
> >  src/couchdb/couch_config.erl        |   11 ++++-------
> >  src/couchdb/couch_config_writer.erl |    8 +++++---
> >  src/couchdb/couch_file.erl          |    5 +----
> >  src/couchdb/couch_log.erl           |    2 --
> >  src/couchdb/couch_server_sup.erl    |   14 +++++++-------
> >  5 files changed, 17 insertions(+), 23 deletions(-)
> > ----------------------------------------------------------------------
> >
> >
> >
> http://git-wip-us.apache.org/repos/asf/couchdb/blob/48371335/src/couchdb/couch_config.erl
> > ----------------------------------------------------------------------
> > diff --git a/src/couchdb/couch_config.erl b/src/couchdb/couch_config.erl
> > index f669853..44a102d 100644
> > --- a/src/couchdb/couch_config.erl
> > +++ b/src/couchdb/couch_config.erl
> > @@ -187,13 +187,10 @@ parse_ini_file(IniFile) ->
> >     case file:read_file(IniFilename) of
> >         {ok, IniBin0} ->
> >             IniBin0;
> > -        {error, eacces} ->
> > -            throw({file_permission_error, IniFile});
> > -        {error, enoent} ->
> > -            Fmt = "Couldn't find server configuration file ~s.",
> > -            Msg = ?l2b(io_lib:format(Fmt, [IniFilename])),
> > -            ?LOG_ERROR("~s~n", [Msg]),
> > -            throw({startup_error, Msg})
> > +        {error, Reason} = Error ->
> > +            ?LOG_ERROR("Couldn't read server configuration file ~s: ~s",
> > +                [IniFilename, file:format_error(Reason)]),
> > +            throw(Error)
> >     end,
> >
> >     Lines = re:split(IniBin, "\r\n|\n|\r|\032", [{return, list}]),
> >
> >
> http://git-wip-us.apache.org/repos/asf/couchdb/blob/48371335/src/couchdb/couch_config_writer.erl
> > ----------------------------------------------------------------------
> > diff --git a/src/couchdb/couch_config_writer.erl
> b/src/couchdb/couch_config_writer.erl
> > index decd269..3a65c37 100644
> > --- a/src/couchdb/couch_config_writer.erl
> > +++ b/src/couchdb/couch_config_writer.erl
> > @@ -22,6 +22,8 @@
> >
> >  -export([save_to_file/2]).
> >
> > +-include("couch_db.hrl").
> > +
> >  %% @spec save_to_file(
> >  %%           Config::{{Section::string(), Option::string()},
> Value::string()},
> >  %%           File::filename()) -> ok
> > @@ -38,9 +40,9 @@ save_to_file({{Section, Key}, Value}, File) ->
> >     case file:write_file(File, NewFileContents) of
> >     ok ->
> >         ok;
> > -    {error, eacces} ->
> > -        {file_permission_error, File};
> > -    Error ->
> > +    {error, Reason} = Error ->
> > +        ?LOG_ERROR("Couldn't write config file ~s: ~s",
> > +            [File, file:format_error(Reason)]),
> >         Error
> >     end.
> >
> >
> >
> http://git-wip-us.apache.org/repos/asf/couchdb/blob/48371335/src/couchdb/couch_file.erl
> > ----------------------------------------------------------------------
> > diff --git a/src/couchdb/couch_file.erl b/src/couchdb/couch_file.erl
> > index e195db0..5e476af 100644
> > --- a/src/couchdb/couch_file.erl
> > +++ b/src/couchdb/couch_file.erl
> > @@ -58,10 +58,7 @@ open(Filepath, Options) ->
> >             {trap_exit, true} -> receive {'EXIT', Pid, _} -> ok end;
> >             {trap_exit, false} -> ok
> >             end,
> > -            case Error of
> > -            {error, eacces} -> {file_permission_error, Filepath};
> > -            _ -> Error
> > -            end
> > +            Error
> >         end;
> >     Error ->
> >         Error
> >
> >
> http://git-wip-us.apache.org/repos/asf/couchdb/blob/48371335/src/couchdb/couch_log.erl
> > ----------------------------------------------------------------------
> > diff --git a/src/couchdb/couch_log.erl b/src/couchdb/couch_log.erl
> > index 8e24cab..7fb95a7 100644
> > --- a/src/couchdb/couch_log.erl
> > +++ b/src/couchdb/couch_log.erl
> > @@ -89,8 +89,6 @@ init([]) ->
> >     case file:open(Filename, [append]) of
> >     {ok, Fd} ->
> >         {ok, #state{fd = Fd, level = Level, sasl = Sasl}};
> > -    {error, eacces} ->
> > -        {stop, {file_permission_error, Filename}};
> >     Error ->
> >         {stop, Error}
> >     end.
> >
> >
> http://git-wip-us.apache.org/repos/asf/couchdb/blob/48371335/src/couchdb/couch_server_sup.erl
> > ----------------------------------------------------------------------
> > diff --git a/src/couchdb/couch_server_sup.erl
> b/src/couchdb/couch_server_sup.erl
> > index 7baede3..be3c3a3 100644
> > --- a/src/couchdb/couch_server_sup.erl
> > +++ b/src/couchdb/couch_server_sup.erl
> > @@ -46,7 +46,9 @@ start_server(IniFiles) ->
> >     {ok, [PidFile]} ->
> >         case file:write_file(PidFile, os:getpid()) of
> >         ok -> ok;
> > -        Error -> io:format("Failed to write PID file ~s, error: ~p",
> [PidFile, Error])
> > +        {error, Reason} ->
> > +            io:format("Failed to write PID file ~s: ~s",
> > +                [PidFile, file:format_error(Reason)])
> >         end;
> >     _ -> ok
> >     end,
> > @@ -121,12 +123,10 @@ start_server(IniFiles) ->
> >             end end || Uri <- Uris],
> >         case file:write_file(UriFile, Lines) of
> >         ok -> ok;
> > -        {error, eacces} ->
> > -            ?LOG_ERROR("Permission error when writing to URI file ~s",
> [UriFile]),
> > -            throw({file_permission_error, UriFile});
> > -        Error2 ->
> > -            ?LOG_ERROR("Failed to write to URI file ~s: ~p~n",
> [UriFile, Error2]),
> > -            throw(Error2)
> > +        {error, Reason2} = Error ->
> > +            ?LOG_ERROR("Failed to write to URI file ~s: ~s",
> > +                [UriFile, file:format_error(Reason2)]),
> > +            throw(Error)
> >         end
> >     end,
> >
> >
>
>
>
> --
> Filipe David Manana,
>
> "Reasonable men adapt themselves to the world.
>  Unreasonable men adapt the world to themselves.
>  That's why all progress depends on unreasonable men."
>

--047d7b33db50bc57b404bb9cca2e--