Task-server Refactor #37
Conversation
There was a problem hiding this comment.
That’s a first quick review. I think we can remove a lot from api/store/*, we don’t need replications and access settings. Probably it would be simpler to just add @hoodie/store-server as dependency and the require('@hoodie/store-server/api') and use that internally? Sorry I’m not sure if we discussed that idea yet.
Let me know if you have any question. We can also go trough step by step, get this PR merged first and then build upon it? In that case I would suggest to create a new branch and send the PR against it instead of master?
| @@ -7,8 +7,7 @@ notifications: | |||
| email: false | |||
| node_js: | |||
| - 4 | |||
| before_script: | |||
| - npm prune | |||
There was a problem hiding this comment.
can you leave the npm prune in? If we don’t have it in store-server, can you send a PR for it? As we cache the node_modules folder, this can save us lots of headaches
| [](https://coveralls.io/r/hoodiehq/hoodie-task-server?branch=master) | ||
| [](https://david-dm.org/hoodiehq/hoodie-task-server) | ||
| [](https://david-dm.org/hoodiehq/hoodie-task-server#info=devDependencies) | ||
| `hoodie-task-server` is a [Hapi](http://hapijs.com/) plugin that exposes an API for managing tasks. |
There was a problem hiding this comment.
it also exposes routes. Maybe that’s what you mean with API, but maybe we can clarify this? To me the API is what the plugin exposes at server.plugins.task.api similar to what store-server and account-server do.
I would mention that hoodie-task-server also defines all necessary CouchDB replication routes so that task objects can be synced, maybe link to somewhere in the CouchDB docs where they are documented?
|
|
||
| ```js | ||
| var Hapi = require('hapi') | ||
| var hapiTask = require('hoodie-task-server') | ||
| var hoodietask = require('@hoodie/task-server') |
|
|
||
| ```js | ||
| options: { | ||
| PouchDB: require('pouchdb-core').plugin('pouchdb-adapter-leveldb') |
There was a problem hiding this comment.
require('pouchdb-core').plugin('pouchdb-adapter-leveldb') won’t work, this happens to me all the time. What you want is require('pouchdb-core').plugin(require('pouchdb-adapter-leveldb')). I think we have to add pouchdb-replication and pouchdb-mapreduce to it
| ```js | ||
| options: { | ||
| PouchDB: require('pouchdb-core') | ||
| .plugin('pouchdb-adapter-http') |
There was a problem hiding this comment.
see above, .plugin does not accept a string but an object
| git clone https://github.com/hoodiehq/hoodie-task-server.git | ||
| cd hoodie-task-server | ||
| cd hoodie-store-server |
|
|
||
| ```js | ||
| // all methods return promises | ||
| task.success(taskDoc) |
There was a problem hiding this comment.
If I recall correctly, we wanted to add an additional property where we can pass in properties that get added to the deleted revision, so we can access them and give more context in the client? So it would be task.success(taskDoc, additionalProperties), additionalProperties being optional
| @@ -0,0 +1,28 @@ | |||
| var Hapi = require('hapi') | |||
| var PouchDB = require('pouchdb-core') | |||
| .plugin('pouchdb-plugin-http') | |||
There was a problem hiding this comment.
.plugin(require('pouchdb-plugin-http'))
also add pouchdb-replication and pouchdb-mapreduce
| @@ -1,45 +1,62 @@ | |||
| { | |||
| "name": "@hoodie/task-server", | |||
| "description": "CouchDB-based REST & front-end API for asynchronous background tasks", | |||
| "description": "Hapi plugin that exposes an API for managing tasks", | |||
| }, | ||
| "repository": { | ||
| "type": "git", | ||
| "url": "https://github.com/hoodiehq/hoodie-task-server.git" | ||
| "url": "git+https://github.com/hoodiehq/hoodie-task-server.git" |
| ``` | ||
|
|
||
| ## How it works | ||
| If you want connect to a CouchDB, use the `pouchdb-adapter-http` and set |
There was a problem hiding this comment.
"If you want connect" should be "If you want to connect"
As discussed here. This is a refactor of the task-server, based on store-server. Further updates are likely necessary, but this should get us going.