Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add partition endpoint for views #1480

Closed
wants to merge 1 commit into from
Closed

add partition endpoint for views #1480

wants to merge 1 commit into from

Conversation

garrensmith
Copy link
Member

Add partition endpoints for view queries. Support /db/_partition/:partitionkey/_designdoc/name/_view/viewname queries

handle_partition_design_req(#httpd{
path_parts=[_DbName, <<"_partition">>, PartitionKey, _Design, Name, <<"_",_/binary>> = Action | _Rest]
}=Req, Db) ->
DbName = mem3:dbname(couch_db:name(Db)),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

couldn't we just use DbName from path_parts above?

}=Req, Db) ->
DbName = mem3:dbname(couch_db:name(Db)),
case mem3:is_partitioned(DbName) of
false -> throw(<<"Database is not partitioned">>);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will be a 500 Server Error? I think you need a different shape to get a 400 Bad Request.

@wohali
Copy link
Member

wohali commented Aug 1, 2018

I know this is for user-specified partitions, but I wonder what endpoint we're exposing for per-shards at :5984 (necessary to deprecate :5986 once and for all), and whether this endpoint will cause confusion.

@wohali wohali closed this Aug 1, 2018
@wohali
Copy link
Member

wohali commented Aug 1, 2018

08:48 <garren> Wohali: I don't quite follow what you mean here
               https://github.com/apache/couchdb/pull/1480#issuecomment-409554240
08:51 <-- ASFBot (~ASFBot@apache/bot/asfbot) has quit (Ping timeout: 260 seconds)
08:51 <@Wohali> I mean, rnewson and i have had a long standing discussiona bout
                introducing a new endpoint on 5984 for shard-level queries
08:52 <@Wohali> we haven't been able to come up with the right endpoint yet
08:52 <@Wohali> but it eventually needs to exist
08:52 <@Wohali> and i just want to ensure that by introducing the new concept of a
                _partition we aren't further confusing people
08:52 <@Wohali> because I wouldn't be surprised to see an endpoint like
                /db/_shard/000000-1ffffff/...
08:53 <@Wohali> and then the question becomes: is this a db partition, or a view partition
                you are proposing?
08:53 <@Wohali> if the latter, then why not /db/_ddoc/name/_view/viewname/_partition
08:53 <@Wohali> i'm trying not to bikeshed, but our ui is already somewhat complex, and
                it's only going to get worse, that's all
08:53 <@Wohali> does that make sense?
08:55 <garren> Yes that makes sense thanks.
08:55 <@Wohali> i 100% guarantee you that people will not understand the difference
                between a db shard and a db partition if we introduce both concepts
                without careful thought :)
08:55 <garren> THe one thing is that a shard can hold multiple partitions. So they are
               related but not that related to shards
08:55 <@Wohali> subtle.
08:55 <garren> Yeah I agree 100%
08:57 <@rnewson> hasn't the partition proposal been on dev@ yet?
08:57 <@rnewson> cause I'm deep into implementation at this point and so is garren
08:57 <@Wohali> changing the endpoint is minor
08:57 <@Wohali> i'm fine with the concept
08:58 <@Wohali> i am not happy with the partition being at the 2nd level in the route,
                though
08:58 <@Wohali> i am not happy with the partition being at the 2nd level in the route,
                though
08:58 <@rnewson> can you respond to the thread saying so then?
08:58 <@Wohali> yes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants