You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by iilyak <gi...@git.apache.org> on 2016/04/20 17:50:03 UTC
[GitHub] couchdb-chttpd pull request: Inject _design/_auth on auth_cache st...
GitHub user iilyak opened a pull request:
https://github.com/apache/couchdb-chttpd/pull/115
Inject _design/_auth on auth_cache startup
We send start_listener event from init to make sure we call
ensure_auth_ddoc_exists as soon as we can. Rather then calling it on
listener restart.
COUCHDB-3000
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/cloudant/couchdb-chttpd 3000-inject_auth_ddoc
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/couchdb-chttpd/pull/115.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 #115
----
commit c8f3497bfa7eb5d30dcddb014772dcd63cd68286
Author: ILYA Khlopotov <ii...@ca.ibm.com>
Date: 2016-04-20T15:45:42Z
Inject _design/_auth on auth_cache startup
We send start_listener event from init to make sure we call
ensure_auth_ddoc_exists as soon as we can. Rather then calling it on
listener restart.
COUCHDB-3000
----
---
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-chttpd pull request #115: Inject _design/_auth on auth_cache startup
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/couchdb-chttpd/pull/115
---
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-chttpd pull request: Inject _design/_auth on auth_cache st...
Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/115#discussion_r60446470
--- Diff: src/chttpd_auth_cache.erl ---
@@ -95,7 +95,8 @@ get_from_cache(UserName) ->
%% gen_server callbacks
init([]) ->
- {ok, #state{changes_pid = spawn_changes(0)}}.
+ self() ! {start_listener, 0},
--- End diff --
There are five ways to deal with delayed initialization.
1. send a message to itself and handle it in handle_info.
2. do gen_server:cast to itself.
3. return `{ok, State, 0}` and implement `handle_info(timeout, ..`
4. use `proc_lib:start_link -> proc_lib:init_ack -> gen_server:enter_loop`
In this particular case we don't have any functions which use this gen_server (handle_call and handle_cast are empty and handle_info handles one specific case which wouldn't be a problem). So we don't care that someone would call us while we are not having changes_pid set. This means that `#4` is overkill for sure. Since we already handle `start_listener` event in handle_info it is easiest to use approach `#1`.
In cases when there are none empty handle_call or handle_cast and gen_server is anonymous I usually choose `#2` (i.e. `gen_server:cast(self(), init)`). Since nobody would be able to call us anyway because our gen_server doesn't have name and pid is not known to the potential caller. By the time we provide the pid to the potential caller init message would be in the process mailbox (because init is executed in the context of that process). Therefore the very first message gen_server will be processing would be `init` message. I would never use `#3` since it does rely on `timeout` atom which doesn't let you to clarify you intend. `#4` is only useful for named processes. Because in this case you might get messages from potential callers before you place `init` message into process mailbox.
---
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-chttpd pull request: Inject _design/_auth on auth_cache st...
Posted by davisp <gi...@git.apache.org>.
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/115#issuecomment-216667074
Right, I'm just saying there's no reason to prefer it over sending a message to self.
---
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-chttpd pull request: Inject _design/_auth on auth_cache st...
Posted by davisp <gi...@git.apache.org>.
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/115#issuecomment-216585377
+1 I wouldn't bother with gen_server:cast/2 as its nothing more than self() ! {'$gen_cast', Msg} which is just adding unnecessary code.
---
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-chttpd pull request: Inject _design/_auth on auth_cache st...
Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/115#discussion_r60447509
--- Diff: src/chttpd_auth_cache.erl ---
@@ -95,7 +95,8 @@ get_from_cache(UserName) ->
%% gen_server callbacks
init([]) ->
- {ok, #state{changes_pid = spawn_changes(0)}}.
+ self() ! {start_listener, 0},
--- End diff --
Frankly I would choose `#2` in this case as well but I would have to refactor the code a little bit. This would slow down the review and would require writing a test suite.
---
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-chttpd pull request: Inject _design/_auth on auth_cache st...
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/115#discussion_r60435956
--- Diff: src/chttpd_auth_cache.erl ---
@@ -95,7 +95,8 @@ get_from_cache(UserName) ->
%% gen_server callbacks
init([]) ->
- {ok, #state{changes_pid = spawn_changes(0)}}.
+ self() ! {start_listener, 0},
--- End diff --
I don't have a lot experience, but I thought that sending a message to self on init is sort of a hack that may cause various issues like hanging and so. Is that true and safe to use here?
---
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-chttpd pull request: Inject _design/_auth on auth_cache st...
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/115#discussion_r60452722
--- Diff: src/chttpd_auth_cache.erl ---
@@ -95,7 +95,8 @@ get_from_cache(UserName) ->
%% gen_server callbacks
init([]) ->
- {ok, #state{changes_pid = spawn_changes(0)}}.
+ self() ! {start_listener, 0},
--- End diff --
Thanks for detailed explanation!
I'm agree with you that all cases that involves dependency on time shouldn't be used. The `#2` sounds good as it will require test suite (tests are good in any case), but I don't have any solid expertise here to judge.
---
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-chttpd pull request: Inject _design/_auth on auth_cache st...
Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/115#discussion_r60447087
--- Diff: src/chttpd_auth_cache.erl ---
@@ -95,7 +95,8 @@ get_from_cache(UserName) ->
%% gen_server callbacks
init([]) ->
- {ok, #state{changes_pid = spawn_changes(0)}}.
+ self() ! {start_listener, 0},
--- End diff --
I forgot to mention `#5` which I saw in some projects.
`erlang:send_after(10, self(), init).`
I wouldn't use it.
---
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-chttpd pull request: Inject _design/_auth on auth_cache st...
Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/115#issuecomment-216630486
`self() ! {'$gen_cast', Msg}` - is implementation detail of OTP which shouldn't be relied upon. The public interface of gen_server module is `gen_server:cast/2`.
---
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.
---