search

camomile-project / camomile-client-javascript

Javascript client for Camomile REST API
http://camomile-project.github.io/
MIT License
1 stars 6 forks source link

simplify watch api, solve #12 #13

Closed rom1504 closed 8 years ago

rom1504 commented 8 years ago

However the unwatch functions are still returned by the watch functions unlike the python api that directly expose the unwatch functions. Would it be better to do the same as the python API or keep the current behavior ?

hbredin commented 8 years ago

I am not sure I understand how the "unwatch" thing works. How can we use this in practice? As of now, it looks like the cancelWatcher can only be called from the then part of the watch promise -- it does not really makes sense to me.

However, I like very much the idea of having watch return the cancelWatcher function. I guess we should think of doing the same in Python client.

rom1504 commented 8 years ago

How can we use this in practice?

The function passed in the then could store the unwatch function somewhere so that it could be called when an unwatch button in a frontend is clicked for example. It's only possible to unwatch when a watch has been done.

hbredin commented 8 years ago

OK. I understand.

Maybe we could automatically store the unwatch function in an attribute of the client instance -- so that the end user does not have to do that. (Because I don't see any other reasonable way this function may be used)

Then, calling client.unwatchCorpus(corpus_id) would run this (internally stored) function, if and only if it exists. What do you think?

rom1504 commented 8 years ago

The only reason the unwatch is returned instead of just exposed (as client.unwatchCorpus) is so it's not necessary to provide the corpus_id when calling it. It means calling unwatch() instead of client.unwatchCorpus(corpus_id).

It could just be possible to expose client.unwatchCorpus(corpus_id) like the python api, and not return the unwatch function.

rom1504 commented 8 years ago

I exposed the unWatch methods as we said. I think it's ready to merge.

hbredin commented 8 years ago

To be consistent with Python client, can you please use unwatchCorpus instead of unWatchCorpus?

rom1504 commented 8 years ago

It's done