Allow ChangeNotifier to use a bound database

[gonvaled]

The ChangeNotifier class currently assumes that it is receiving as parameter a non-bound database.
If that is not the case, it fails.
This patch supports both cases: bound and non-bound databases. A bound database is recognized by ChangeNotifier because the dbName is None.

Ideally, this should be handled inside CouchDB (in client.py), which is the one who really knows whether the class has been bound to a database or not.

The patch also moves the url generation to the CouchDb class, method changesUrl. The idea is that the knowledge about the database structure should be centralized in the CouchDB class. This changesUrl method can be extended to generate other URLs if needed.

[thomasvs]

Let me start off by saying that I think the binding to a name functionality in CouchDB is horrendous to me. I don't like that kind of magic. It's hard to document properly, it's unnecessary magic, it just shows that Python lets you shoot yourself in the foot if you really want to, and it doesn't solve any problem. In addition, it leads to terrible if's like you just had to put in there. Your constructor 'assumes' something based on whether a parameter is set. This something is questionable functionality, but in any case it's the CouchDB class that knows whether it's bound or not. In addition, this condition can change over the lifetime of CouchDB, and thus can change after the ChangeNotifier was created.

I am personally in favour of not making ChangeNotifier work with the bound case.

I'm also in favour of dropping support for binding altogether in the interest of readability and maintainability. Please open a discussion on the dev list for that.

[gonvaled]

I agree with your point of view. It would even be better to have the bind method just track internally the database with which we are trying to bind (self._db_name for example), but not change the API. That is, all database access methods would never expect the database name: binding would be the first operation, always required: first we bind to a database, and then we issue requests to it. This has the advantage that we are "bound" (so the database name must not be passed around all the time), but the API is stable.

The other option is to just completely get rid of the binding mechanism, which is indeed confusing. The database name must then be always explicitly given.

[gonvaled]

What do you think about the second part of the commit:

The patch also moves the url generation to the CouchDb class, method changesUrl. The idea is that the knowledge about the database structure should be centralized in the CouchDB class. This changesUrl method can be extended to generate other URLs if needed.

[objcode]

I'm not opposed to making CouchDB be the expert on urls.

Looking at the rest of the diff, it probably makes more sense to undo the partial binding that is currently done and instead make dbname a kwarg parameter that gets replaced inside the function with self.dbName if it's None.

Gonvaled, do you want to take a look at this refactor of the client API?

As a general note, this pull request would need a test showing the new behavior works before I'd merge it.

[gonvaled]

Hi Sean,

Not sure that is the way you meant it to be: I have defined a default parameter (dbName=None) in some functions. If no value is given (it is None), then the instance dbName is used (which has been provided during client creation). If that is also None, it will of course fail (what to do in this case?)

I have not use a kwarg parameter, since I thought this implementation is easy enough.

This will have effect in old code which uses explicitly dbName, since the location of this positional parameter has changed (since it has a default value, it can not occur before other parameters without default value). The functions affected by this parameter location change are (all in client.py): openDoc, saveDoc, deleteDoc, openView and tempView.

The clients which were already using bound databases will see no difference in the api.