Github user davisp commented on the pull request:
https://github.com/apache/couchdb-couch/pull/1#issuecomment-35115035
There's a problem with the waiting queue not being per-language here. This
would block some clients of idle languages if one of the languages happens to
reac
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-couch/pull/1#issuecomment-35115764
Beyond the queue-per-language question the diff is mostly good for me. I'd
like to see some deduplication in logic but beyond that it looks fine.
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-couch/pull/1#discussion_r9772933
First, replace this with couch_util:to_binary/1, second, instead of calling
it all over the place, just add it into the handle_call clauses so that all of
the Lang
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-couch/pull/1#discussion_r9772943
You can't use os:timestamp/0 here becuase its not guaranteed to be unique.
You'll need to use erlang:now/0.
If your project is set up for it, you can rep
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-couch/pull/1#issuecomment-35157662
I think it'd be good to clean up the {From, Lang} vs. {From, Lang, DDoc,
DDocKey} mess. Lets add a record `#client{timestamp, lang, ddoc, ddockey}`
record and
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-couch/pull/1#discussion_r9800282
Slight nit but for live introspection this is a bit of an awkward name. I'd
go with couch_proc_manage_waiting. Might also add it as a -define constant for
line l
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-couch/pull/1#issuecomment-35310894
+1 after renaming the table.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please top
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-couch/pull/1#issuecomment-35312807
Merged into 1843-feature-bigcouch
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-mem3/pull/1#issuecomment-46502194
+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
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/16#issuecomment-49918363
+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
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-couch-stats/pull/1#issuecomment-52672909
+1. We'll need to start rebasing/merging all of the changes to the other
repos soon as this lands because of the module name conflicts with
couchdb-
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-couch-replicator/pull/2#issuecomment-53862268
Can you link to the corresponding PR to couchdb-couch to help with the
review?
---
If your project is set up for it, you can reply to this email and
Github user davisp commented on the pull request:
https://github.com/apache/couchdb/pull/261#issuecomment-54515877
+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
GitHub user davisp opened a pull request:
https://github.com/apache/couchdb-couch/pull/5
2322 fix couch proc manager limits
This fixes the couch_proc_manager limit counting by rearranging the
increment and decrements when processes are created and destroyed. It ensures
that each
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-couch/pull/5#issuecomment-54691882
I should note that all eunit and JS tests passed. I had to run the
delayed_commits in isolation once to get it green but I'm told its rather flaky
so likely n
Github user davisp commented on the pull request:
https://github.com/apache/couchdb/pull/265#issuecomment-60129897
+1 once you remove your URL mangling
---
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
Github user davisp commented on a diff in the pull request:
https://github.com/apache/couchdb-couch-mrview/pull/2#discussion_r19233379
--- Diff: src/couch_mrview.erl ---
@@ -135,11 +140,67 @@ query_view(Db, {Type, View, Ref}, Args, Callback,
Acc) ->
erlang:demoni
Github user davisp commented on a diff in the pull request:
https://github.com/apache/couchdb-couch-mrview/pull/2#discussion_r19233446
--- Diff: src/couch_mrview.erl ---
@@ -148,11 +209,51 @@ get_info(Db, DDoc) ->
trigger_update(Db, DDoc) ->
trigger_update(Db
Github user davisp commented on a diff in the pull request:
https://github.com/apache/couchdb-couch-mrview/pull/2#discussion_r19233515
--- Diff: src/couch_mrview.erl ---
@@ -148,11 +209,51 @@ get_info(Db, DDoc) ->
trigger_update(Db, DDoc) ->
trigger_update(Db
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-couch-mrview/pull/2#issuecomment-60130901
I've given up on commenting on indentation issues individually. That said,
there's lots and lots that needs to be fixed.
---
If your project is set
Github user davisp commented on a diff in the pull request:
https://github.com/apache/couchdb-couch-mrview/pull/2#discussion_r19233778
--- Diff: src/couch_mrview_changes.erl ---
@@ -0,0 +1,18 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); yo
Github user davisp commented on a diff in the pull request:
https://github.com/apache/couchdb-couch-mrview/pull/2#discussion_r19234258
--- Diff: src/couch_mrview_http.erl ---
@@ -46,7 +48,46 @@ handle_all_docs_req(#httpd{method='POST'}=Req, Db) ->
handle_all_docs
Github user davisp commented on a diff in the pull request:
https://github.com/apache/couchdb-couch-mrview/pull/2#discussion_r19234314
--- Diff: include/couch_mrview.hrl ---
@@ -18,12 +18,14 @@
idx_name,
language,
design_opts=[],
+seq_indexed
Github user davisp commented on a diff in the pull request:
https://github.com/apache/couchdb-couch-mrview/pull/2#discussion_r19234375
--- Diff: src/couch_mrview_http.erl ---
@@ -46,7 +48,46 @@ handle_all_docs_req(#httpd{method='POST'}=Req, Db) ->
handle_all_docs
Github user davisp commented on a diff in the pull request:
https://github.com/apache/couchdb-couch-mrview/pull/2#discussion_r19234403
--- Diff: src/couch_mrview_http.erl ---
@@ -438,8 +479,10 @@ parse_boolean(true) ->
true;
parse_boolean(false) ->
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-couch-mrview/pull/2#issuecomment-60133065
src/couch_mrview_update_notifier.erl is based on legacy code. That should
pipe through couch_event now.
---
If your project is set up for it, you can reply
Github user davisp commented on a diff in the pull request:
https://github.com/apache/couchdb-couch-mrview/pull/2#discussion_r19235117
--- Diff: src/couch_mrview_updater.erl ---
@@ -210,69 +254,122 @@ accumulate_more(NumDocIds) ->
andalso CurrMem < list_to_i
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-couch-mrview/pull/2#issuecomment-60137836
Beyond the style issues there's some awfully confusing logic embedded in
this PR. A lot of things could be simplified if we pull things out into
func
Github user davisp commented on a diff in the pull request:
https://github.com/apache/couchdb-couch/pull/4#discussion_r19236997
--- Diff: src/couch_changes.erl ---
@@ -332,24 +444,52 @@ build_acc(Args, Callback, UserAcc, Db, StartSeq,
Prepend, Timeout, TimeoutFun
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-couch/pull/4#issuecomment-60138640
This change creates a cyclic dependency between couch_mrview and couch
which is ungood. Its not necessarily a show stopper but I'd really like to put
some th
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-couch/pull/11#issuecomment-60799747
+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
Github user davisp commented on the pull request:
https://github.com/apache/couchdb/pull/280#issuecomment-62440216
+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
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-couch/pull/15#issuecomment-62440626
Do we want to worry about backwards compatibility here or not? Or maybe we
can add a check that throws a 400 if it detects an old header name? This just
feels like
Github user davisp commented on the pull request:
https://github.com/apache/couchdb/pull/281#issuecomment-62440688
+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
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-couch/pull/15#issuecomment-62443228
@kxepal Sure, I agree that normalizing things to be consistent is a good
idea. I'm just saying that for any user that happens to miss this particular
change i
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/8#issuecomment-62443600
I'd agree with @chewbranca here. There's a significant amount of
duplication between the two HTTP layers that everyone agrees needs to be
removed. I d
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-couch-mrview/pull/9#issuecomment-62804143
+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
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-couch/pull/17#issuecomment-65110606
+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
GitHub user davisp opened a pull request:
https://github.com/apache/couchdb-couch/pull/21
Refactor couch_httpd_auth's AuthModule construct
We were missing the change necessary to upgrade user documents with the
move to pbkdf2 in a cluster. This also adds the ability f
GitHub user davisp opened a pull request:
https://github.com/apache/couchdb-chttpd/pull/13
Update chttpd_auth_cache for new couch_auth_cache
This updates the interface to chttpd_auth_cache to match the new
callback API expected by couch_httpd_auth. Specifically this allows for
Github user davisp commented on a diff in the pull request:
https://github.com/apache/couchdb-couch/pull/21#discussion_r21389526
--- Diff: src/couch_auth_cache.erl ---
@@ -42,12 +43,18 @@
-spec get_user_creds(UserName::string() | binary()) ->
-Credenti
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-couch/pull/19#issuecomment-66406876
There be dragons in this code so I'd like to hold off until someone can
find time to make sure there are zero behavior changes. I've fixed a number of
bu
Github user davisp commented on a diff in the pull request:
https://github.com/apache/couchdb-couch/pull/22#discussion_r21720537
--- Diff: src/couch_db_updater.erl ---
@@ -985,27 +985,18 @@ sync_header(Db, NewHeader) ->
waiting_delayed_commit=
Github user davisp commented on a diff in the pull request:
https://github.com/apache/couchdb-couch/pull/22#discussion_r21720525
--- Diff: src/couch_db_updater.erl ---
@@ -985,27 +985,18 @@ sync_header(Db, NewHeader) ->
waiting_delayed_commit=
Github user davisp commented on a diff in the pull request:
https://github.com/apache/couchdb-couch/pull/22#discussion_r21720641
--- Diff: src/couch_db_updater.erl ---
@@ -1044,10 +1057,11 @@ copy_docs(Db, #db{fd = DestFd} = NewDb, MixedInfos,
Retry) ->
A =
Github user davisp commented on a diff in the pull request:
https://github.com/apache/couchdb-couch/pull/22#discussion_r21720818
--- Diff: src/couch_db_updater.erl ---
@@ -1061,22 +1075,22 @@ copy_docs(Db, #db{fd = DestFd} = NewDb, MixedInfos,
Retry
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-couch/pull/22#issuecomment-66713683
Good work on this. I hadn't even contemplated the deduplicating between
documents. That approach looks like it'll work quite nicely. The only major
thing
GitHub user davisp opened a pull request:
https://github.com/apache/couchdb-fabric/pull/6
Add system db handlers to fabric_doc_update
This hard codes the before_doc_write functions for replicator and user
databases like they are in couch_server. This is mostly motivated by the
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-fabric/pull/6#issuecomment-66816904
Yeah, the is_replicator_db and is_users_db should probably be in
couch_replicator and couch_users_db but I figured we can clean that up later if
need be
GitHub user davisp opened a pull request:
https://github.com/apache/couchdb-couch/pull/23
Add IS_OLD_RECORD macro
This can be used in guards to detect if a state variable needs to be
upgraded. An example of it's use might be:
handle_call(Msg, From, St)
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-fabric/pull/5#issuecomment-66831488
+1 Merging.
---
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
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-fabric/pull/5#issuecomment-66831827
Just noticed that the branch name is wrong. Can you push a new PR with the
proper naming scheme please?
---
If your project is set up for it, you can reply to this
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/14#issuecomment-66832337
This needs a new branch name as well.
---
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
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-fabric/pull/7#issuecomment-67229541
+1. Merging
---
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
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/15#issuecomment-67229579
+1 merging
---
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
Github user davisp commented on a diff in the pull request:
https://github.com/apache/couchdb-couch/pull/23#discussion_r21931145
--- Diff: include/couch_db.hrl ---
@@ -23,6 +23,8 @@
-define(JSON_ENCODE(V), couch_util:json_encode(V)).
-define(JSON_DECODE(V
Github user davisp commented on a diff in the pull request:
https://github.com/apache/couchdb-couch/pull/24#discussion_r21931536
--- Diff: src/couch_db_updater.erl ---
@@ -676,29 +676,43 @@ flush_trees(#db{fd = Fd} = Db,
_ ->
{Value, Sizes
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-couch/pull/24#issuecomment-67239293
That looks pretty good but there's still an issue with the counting of
active size. Unfortunately I think the fix is going to require us to start
using @strm
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-couch/pull/24#issuecomment-69077741
For posterity, we were trying to make this work without requiring an
on-disk format change but sadly that appears to not be possible due to the lack
of information
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-jiffy/pull/1#issuecomment-85244644
For the update to 0.13 we should just sync from upstream. As has been
pointed out if we commit to master on any of these we permanently break our
ability to track
Github user davisp commented on the pull request:
https://github.com/apache/couchdb/pull/304#issuecomment-85245732
How do we get this closed if the patch already landed?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If
Github user davisp commented on the pull request:
https://github.com/apache/couchdb/pull/288#issuecomment-85247064
LGTM
---
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
Github user davisp commented on the pull request:
https://github.com/apache/couchdb/pull/272#issuecomment-85247682
LGTM
---
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
Github user davisp commented on the pull request:
https://github.com/apache/couchdb/pull/270#issuecomment-85248702
Erm, so +1 to merging this.
---
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
Github user davisp commented on the pull request:
https://github.com/apache/couchdb/pull/270#issuecomment-85248649
You can add that ./dev/run -n 1 will spin up a single node as well.
Obviously that's just a single node running the full clustered interface. If
anyone feels like m
Github user davisp commented on the pull request:
https://github.com/apache/couchdb/pull/258#issuecomment-85250241
This seems like it'd be better to write this as a plugin.
For starters, randomly loading a module named sd_notify seems not good from
a security point of
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, [
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, [
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, yo
Github user davisp commented on the pull request:
https://github.com/apache/couchdb/pull/122#issuecomment-85254157
Its fixed on the clustered interface though we'll may want to port this to
the couch_httpd.erl in couchdb-couch.git
---
If your project is set up for it, you can
Github user davisp commented on the pull request:
https://github.com/apache/couchdb/pull/98#issuecomment-85254598
I'm pretty sure the underlying bug is fixed in 2.0. Might want to revisit
the docs on this.
---
If your project is set up for it, you can reply to this email and
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/29#issuecomment-85257224
LGTM
---
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
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/28#issuecomment-85257576
Voting 0 with no modifier as per same reason as @janl and @rnewson. Also
agree using a comma instead of the slash when listing the possible values.
---
If your
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/24#issuecomment-85260053
LGTM
---
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
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/26#issuecomment-85269588
The PR tile, linked ticket, and diff don't seem to match to me. The diff
appears to change the error message to include the database name (which is
fine,
Github user davisp commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/12#discussion_r26996242
--- Diff: src/chttpd_auth_cache.erl ---
@@ -91,6 +102,12 @@ handle_info({'DOWN', _, _, Pid, Reason},
#state{changes_pid=Pi
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/12#issuecomment-85272031
+1 to squerge
---
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
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/11#issuecomment-85272309
+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
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-mem3/pull/3#issuecomment-85275680
Looks like this is missing adding the ets_lru worker to the mem3
supervision tree.
---
If your project is set up for it, you can reply to this email and have your
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-couch/pull/29#issuecomment-85280210
+1 after squashing the refactor commit
---
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
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-couch/pull/28#issuecomment-85280587
Which dependencies in priv? *.d seems a bit overly general, perhaps limit
to the directory they affect?
---
If your project is set up for it, you can reply to this
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-couch/pull/23#issuecomment-85280780
+1 to merge. I guess I should probably get to that.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well
Github user davisp commented on a diff in the pull request:
https://github.com/apache/couchdb-couch/pull/20#discussion_r26997498
--- Diff: src/couch_server.erl ---
@@ -116,30 +116,37 @@ create(DbName, Options0) ->
delete(DbName, Options) ->
gen_serve
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-couch/pull/20#issuecomment-85281459
I haven't done a thorough review yet, but this seems related to PR #41
---
If your project is set up for it, you can reply to this email and have your
reply a
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-couch-index/pull/3#issuecomment-85283918
+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
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-couch-mrview/pull/14#issuecomment-85285422
+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
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-couch-mrview/pull/15#issuecomment-85285305
+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
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-couch-mrview/pull/11#issuecomment-85285528
+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
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-couch-mrview/pull/5#issuecomment-85285843
+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
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-couch-replicator/pull/8#issuecomment-85286204
Can you link to the PR that changes test_util?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-ets-lru/pull/1#issuecomment-97806286
+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
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/34#issuecomment-104318654
This looks mostly good to me. I'm confused by the changes to add header
manipulation functions in 9ab5b83 which don't appear to be used anywhere. I
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/34#issuecomment-104318793
Ah, found it. Reviewing the others as well.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If
Github user davisp commented on the pull request:
https://github.com/apache/couchdb-couch/pull/51#issuecomment-104322934
You should adding and removing documents in between requests so that you
can verify that the etag changes appropriately.
---
If your project is set up for it, you
Github user davisp commented on a diff in the pull request:
https://github.com/apache/couchdb-couch-mrview/pull/18#discussion_r30815269
--- Diff: src/couch_mrview_util.erl ---
@@ -1016,3 +1019,26 @@ get_view_queries({Props}) ->
_ ->
throw({bad_r
Github user davisp commented on a diff in the pull request:
https://github.com/apache/couchdb-couch-epi/pull/4#discussion_r34695307
--- Diff: src/couch_epi_data.erl ---
@@ -88,7 +88,8 @@ handle_call(stop, _From, State) ->
handle_call(_Request, _From, State) ->
96 matches
Mail list logo
