Leaving database name validation to CouchDB

[Matt Goodall]

Hi all,

I want to remove the database name checks in couchdb-python completely
(see couchdb.client.validate_dbname) and instead leave it to CouchDB
to decide. We're accidentally blocking access to the _replicator
database right now and who knows what internal database names CouchDB
might add in the future.

Unfortunately, getting the descriptive "illegal_database_name" error
back from CouchDB means using GET instead of HEAD. There are only
three places in the code that are really affected. Two are fine IMHO,
one I would like to question to see if I can make it go away ;-) ...

* Server.__contains__
* Database.__nonzero__

Replace HEAD with GET. It should have minimal impact as the response
body is tiny, shouldn't stress CouchDB, and I can't imagine the
methods are called often anyway.

* Server.__getitem__

I suspect this is called quite frequently. I could replace HEAD with
GET and accept the additional overhead. However, I think a much better
solution is to simply drop the HTTP request completely and update the
docstring to remove mention of ResourceNotFound.

Personally, I've always avoided Server.__getitem__ to avoid the extra
HEAD request I didn't want and isn't even useful. I've never written
any useful code that tries to handle ResourceNotFound - I'm going to
find out soon enough if I got the database name wrong and a validly
named database could be deleted at any time anyway.

What are your thoughts?

- Matt

[Dirkjan Ochtman]

On Thu, Nov 17, 2011 at 16:07, Matt Goodall <matt.g...@gmail.com> wrote:
> Unfortunately, getting the descriptive "illegal_database_name" error
> back from CouchDB means using GET instead of HEAD. There are only
> three places in the code that are really affected. Two are fine IMHO,
> one I would like to question to see if I can make it go away ;-) ...

What kind of response do you get for HEAD on an illegal database name?

Cheers,

Dirkjan

[Matt Goodall]

Same status code, but no JSON body so no useful error message:

$ curl http://localhost:5984/_illegal -i
HTTP/1.1 400 Bad Request
Server: CouchDB/1.2.0a-54b9726-git (Erlang OTP/R14B02)
Date: Fri, 18 Nov 2011 10:22:21 GMT
Content-Type: text/plain;charset=utf-8
Content-Length: 181
Cache-Control: must-revalidate

{"error":"illegal_database_name","reason":"Only lowercase characters
(a-z), digits (0-9), and any of the characters _, $, (, ), +, -, and /
are allowed. Must begin with a letter."}

$ curl http://localhost:5984/_illegal -i -I
HTTP/1.1 400 Bad Request
Server: CouchDB/1.2.0a-54b9726-git (Erlang OTP/R14B02)
Date: Fri, 18 Nov 2011 10:22:24 GMT
Content-Type: text/plain;charset=utf-8
Content-Length: 181
Cache-Control: must-revalidate

- Matt

[Dirkjan Ochtman]

On Fri, Nov 18, 2011 at 11:24, Matt Goodall <matt.g...@gmail.com> wrote:
> Same status code, but no JSON body so no useful error message:

Could we just interpret a 400 Bad Request response in
Server.__getitem__ and throw the nice error message we already know
belongs there (or something very close)?

Cheers,

Dirkjan

[Matt Goodall]

I've just looked through the CouchDB source and *currently* that would
work ... I think. However, I really don't like the idea of taking
guesses at errors - if we got it wrong it would be quite misleading.
That would be like a docstring that lies about a function's
implementation ;-).

I've crudely (and I do mean that!) checked the performance of HEAD vs
GET. GET is a bit slower as expected, but it looks minimal. Maybe 1%
slower performing 5000 Database.__nonzero__ calls with no obvious
impact on the CouchDB backend.

Personally, I think I've come to the conclusion that I'm in favour of
retaining the current API and accepting the very minor hit related to
GET vs HEAD.

- Matt

[Dirkjan Ochtman]

On Fri, Nov 18, 2011 at 12:20, Matt Goodall <matt.g...@gmail.com> wrote:
> Personally, I think I've come to the conclusion that I'm in favour of
> retaining the current API and accepting the very minor hit related to
> GET vs HEAD.

Okay, sounds good. :)

Cheers,

Dirkjan

[Steve Ross]

So is there a workaround for getting the replicator url to work?

--
You received this message because you are subscribed to the Google Groups "CouchDB-Python" group.
To post to this group, send email to couchdb...@googlegroups.com.
To unsubscribe from this group, send email to couchdb-pytho...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/couchdb-python?hl=en.

[Matt Goodall]

On 22 November 2011 18:25, Steve Ross <nowh...@gmail.com> wrote:
> So is there a workaround for getting the replicator url to work?

I've got most of a commit ready that leaves the checking to the
server. I even have a unittest that access the _replicator database
:).

I'll try to finish it off and commit tonight.

- Matt

[Steve Ross]

does the replicate function not work? sorry I am bad about not searching the source first.

def replicate(self, source, target, **options):

Steve Ross
web application & interface developer
http://blog.stevensross.com
[mobile] (912) 344-8113
[ AIM / Yahoo! : zeriumsteven ] [googleTalk : nowhiding ]

[Matt Goodall]

On 22 November 2011 18:50, Steve Ross <nowh...@gmail.com> wrote:
> does the replicate function not work? sorry I am bad about not searching the
> source first.
> def replicate(self, source, target, **options):

Oh, yes Server.replicate works fine! Sorry, but for some reason I
thought you wanted to access the actual _replicator database.

- Matt