find() has weird API

[bert]

datastore.find() was recently changed to not only take the query dict but an array of property names to return, too. That property name list should instead be made into a query parameter. Not only does it streamline the API, but it is actually invalid to send the default value (None) via DBus to get all the properties without listing them.

[jg]

Marco, opinion?

[tomeu]

Bert, sending an empty list would return all the properties an entry has.

[bert]

Replying to tomeu:

Bert, sending an empty list would return all the properties an entry has.

Maybe, although that is counter-intuitive (the default parameter in the source is None not an empty list). Anyway, sending in a useless empty list is arguably not as nice as just not adding a parameter to the query.

And what if we want to add an option to exclude specific properties? Add another argument? IMHO having all these things as query parameters makes a whole lot more sense, is easier to document etc. Like, why is limiting the number of items a query parameter, but not the number of properties? And, what do you actually gain by that extra parameter? You can easily use a keyword argument at the calling side if I am not misreading the code.

[tomeu]

Replying to bert:

Replying to tomeu:

Bert, sending an empty list would return all the properties an entry has.

Maybe, although that is counter-intuitive (the default parameter in the source is None not an empty list). Anyway, sending in a useless empty list is arguably not as nice as just not adding a parameter to the query.

I would prefer to do like you say, but you cannot send None through dbus...

And what if we want to add an option to exclude specific properties? Add another argument? IMHO having all these things as query parameters makes a whole lot more sense, is easier to document etc. Like, why is limiting the number of items a query parameter, but not the number of properties? And, what do you actually gain by that extra parameter?

I agree on that.

You can easily use a keyword argument at the calling side if I am not misreading the code.

Not when calling through dbus, AFAIK. Anyway, passing a dictionary like the query param is the same thing.

[smcv]

This appears to have been fixed in Datastore git as of snapshot cb0acdf653 (which caused #4440 in Sugar). Joyride builds from at least 90 onwards have a newer snapshot, so this bug can probably be closed.

[tomeu]

Have reverted that commit, as it's too late for API changes for FRS. Retargeting for 1.1, when we hope to review the DS API.

[smcv]

Keywording, I'd like to keep track of D-Bus API issues.