Use iobackends to send status messages. #145

pardo-bsso · 2014-03-05T14:58:51Z

This also depends on inaes-tic/mbc-common#62

For some tests we do a require of mosto.js, that in turn ends up calling iobackend.patchBackbone(). This wouldn't be a problem if it happens at the beginning but within the test suite models were created before and after that and it is not a very good thing (for instance, Backbone-relational crashes with a 'RangeError: Maximum call stack size exceeded'). So, we patch backbone before all tests are run and also move the require() statements inside the functions. That way they will be evaluated only when they are run and not when the whole test suite is built.

jmrunge · 2014-03-13T15:19:03Z

backends.js

+        dbPort: 27017
+    }
+};
+var test_db = mbc.db(test_conf.db);


Please keep test code inside tests... You can pass db to the constructor of this function and mosto server also receives the db object and can pass it alongside when calling this...

On 13 March 2014 12:19, Juan Martin Runge [email protected] wrote:

In backends.js:

@@ -0,0 +1,85 @@
+var mbc = require('mbc-common'),

search_options = mbc.config.Search,

collections = mbc.config.Common.Collections,

backboneio = require('backbone.io'),

middleware = new mbc.iobackends().get_middleware()
+;

+var test_conf = {

db: {

dbName: 'mediatestdb',

dbHost: 'localhost',

dbPort: 27017

}
+};
+var test_db = mbc.db(test_conf.db);

Please keep test code inside tests... You can pass db to the constructor of this function and mosto server also receives the db object and can pass it alongside when calling this...

Alright. Some tests do require() of mosto.js and create some things
on the db. We can either move the connection info for the test db
somewhere else here (inside mosto) or add it as another config option
and passing that to the call to mbc.db()

Take a look at the comment I made in test/001-patch-backbone.js and let me know what do you think. On mosto.js we could do

if (!opts.db) db = mbc.db(); else db = opts.db;

Or something like this...

jmrunge · 2014-03-13T15:51:57Z

Please make tests for all this.

pardo-bsso · 2014-03-13T20:35:01Z

We should use a testDb for all tests and pass it here to backends_conf to avoid hardcoding....

It is going to be defined somewhere anyways, I'd rather put it on the config.

I still have the problem that I need to call patchBackbone() on an iobackend with all the backends (the ones used by mosto alone and the ones needed for the models created inside the tests) before creating any model as doing otherwise is not possible (mixing patched and unpatched models)

I can extract the backends needed by the tests alone to another file and merge them with the ones from mosto , so things are separated. But the connection info to reach the test db still has to be defined somewhere.

jmrunge · 2014-03-17T14:18:52Z

As for normal usage, the db info comes from config. For tests you could define it in the same file as patchBackbone and fix tests to use this db (mediatestdb) I think

pardo-bsso · 2014-03-19T01:25:51Z

I think we should stick to the initial error until its resolved instead of dropping and creating a new one...

Okay, I can see that being useful with missing media files (for example checking upon startup if they are still missing). But for other things like, timeouts or lost connections? Perhaps I really missed the point but don't we restart melted when starting Mosto?

pardo-bsso · 2014-03-19T01:29:31Z

As for normal usage, the db info comes from config. For tests you could define it in the same file as patchBackbone and fix tests to use this db (mediatestdb) I think

Sorry if I misunderstood your suggestion but I don't agree about saving the connection details to a test db inside iobackends. I'd rather add another config key with that information and if we are running a test just create all the backends with it instead of the default.

pardo-bsso · 2014-03-19T01:31:35Z

hbError is thrown when an error occurs on the syncMelted func, not when its out of sync. noClips, outOfSync, forceCheckout, hbError are valid events to listen for I think.

Got to read better what's going on there. Besides listening for them , are all of them really useful if present on the caspa ui?

pardo-bsso · 2014-03-19T01:34:01Z

1- We should fix thant in melted-node ASAP! reconnect should mean that is already reconnected, connected should be used only when connected first time and disconnected every time it gets disconnected no matter why.
2- I think response-timeout, command-error, connected, disconnected, connection-error, reconnect and disconnect are interesting events to listen for... User will not be looking at the log, will be looking at the UI

Alright, I guess that nobody will complain if we change the event naming.

jmrunge · 2014-03-19T16:35:23Z

2014-03-18 22:25 GMT-03:00 Adrián Pardini [email protected]:

I think we should stick to the initial error until its resolved instead of
dropping and creating a new one...

Okay, I can see that being useful with missing media files (for example
checking upon startup if they are still missing). But for other things
like, timeouts or lost connections? Perhaps I really missed the point but
don't we restart melted when starting Mosto?

Nope. We only start it when not already started...

Reply to this email directly or view it on GitHubhttps://github.com//pull/145#issuecomment-38008763
.

jmrunge · 2014-03-19T16:40:19Z

2014-03-18 22:31 GMT-03:00 Adrián Pardini [email protected]:

hbError is thrown when an error occurs on the syncMelted func, not when
its out of sync. noClips, outOfSync, forceCheckout, hbError are valid
events to listen for I think.

Got to read better what's going on there. Besides listening for them , are
all of them really useful if present on the caspa ui?

hbError: whenever an error occurs during sync check. Like any other error,
its important for the user

noClips: means that melted has run out of clips. Should never happend!
Only if user hasnt load any clips... (rare situation)

outOfSync: well, the name says everything... I think the user would like to
know when happends this (that sould not happend! Only when user changes
playlists within time window)

forceCheckout: Whenever mosto needs more clips, this is just info.

Reply to this email directly or view it on GitHubhttps://github.com//pull/145#issuecomment-38009057
.

jmrunge · 2014-03-19T16:41:59Z

2014-03-18 22:29 GMT-03:00 Adrián Pardini [email protected]:

As for normal usage, the db info comes from config. For tests you could
define it in the same file as patchBackbone and fix tests to use this db
(mediatestdb) I think

Sorry if I misunderstood your suggestion but I don't agree about saving
the connection details to a test db inside iobackends. I'd rather add
another config key with that information and if we are running a test just
create all the backends with it instead of the default.

I didnt ment to save connection details inside iobackends, just to have db
configured before we create them and before patching backbone...

Reply to this email directly or view it on GitHubhttps://github.com//pull/145#issuecomment-38008949
.

xaiki · 2014-03-25T19:39:38Z

On Tue, Mar 18 2014, [email protected] wrote:

I think we should stick to the initial error until its resolved instead of dropping and creating a new one...

Okay, I can see that being useful with missing media files (for example checking upon startup if they are still missing). But for other things like, timeouts or lost connections? Perhaps I really missed the point but don't we restart melted when starting Mosto?
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

yes, but we shouldn't. it's a hack, it needs to go away.

jmrunge · 2014-03-26T00:16:29Z

2014-03-25 16:39 GMT-03:00 Niv Sardi [email protected]:

On Tue, Mar 18 2014, [email protected] wrote:

I think we should stick to the initial error until its resolved instead
of dropping and creating a new one...

Okay, I can see that being useful with missing media files (for example
checking upon startup if they are still missing). But for other things
like, timeouts or lost connections? Perhaps I really missed the point but
don't we restart melted when starting Mosto?
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

yes, but we shouldn't. it's a hack, it needs to go away.

I checked Makefile and code and we dont (or could not find it). We only
start melted if not already started.

Reply to this email directly or view it on GitHubhttps://github.com//pull/145#issuecomment-38610763
.

pardo-bsso · 2014-03-27T01:55:57Z

Moving to #149

pardo-bsso added 13 commits March 2, 2014 18:18

adds new iobackends models

b8537b6

upgrade required Mocha.

0dd5c6c

CaspaDriver: use models to send status over Redis

02ce087

mosto_server: send status messages.

c292b06

mosto_server: use self instead of this on callback

c6dd397

melted-node-driver: relay events from mlt

b9a9df5

melted-node-driver: send error message if getServerStatus() fails.

4a880c8

heartbeats: emit status messages.

003ed90

MeltedCollection: relay events from driver.

ed76552

fix sending of length and current progress.

a192192

use tmpId middleware so Caspa sees new messages.

98a2634

set the fields ProgressStatus defines in the model

f3912f5

jmrunge reviewed Mar 13, 2014
View reviewed changes

pardo-bsso mentioned this pull request Mar 27, 2014

Messages iobackends ultimo #149

Open

pardo-bsso closed this Mar 27, 2014

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use iobackends to send status messages. #145

Use iobackends to send status messages. #145

pardo-bsso commented Mar 5, 2014

jmrunge Mar 13, 2014

pardo-bsso Mar 13, 2014

jmrunge Mar 13, 2014

jmrunge commented Mar 13, 2014

pardo-bsso commented Mar 13, 2014

jmrunge commented Mar 17, 2014

pardo-bsso commented Mar 19, 2014

pardo-bsso commented Mar 19, 2014

pardo-bsso commented Mar 19, 2014

pardo-bsso commented Mar 19, 2014

jmrunge commented Mar 19, 2014

jmrunge commented Mar 19, 2014

jmrunge commented Mar 19, 2014

xaiki commented Mar 25, 2014

jmrunge commented Mar 26, 2014

pardo-bsso commented Mar 27, 2014

Use iobackends to send status messages. #145

Use iobackends to send status messages. #145

Conversation

pardo-bsso commented Mar 5, 2014

jmrunge Mar 13, 2014

Choose a reason for hiding this comment

pardo-bsso Mar 13, 2014

Choose a reason for hiding this comment

jmrunge Mar 13, 2014

Choose a reason for hiding this comment

jmrunge commented Mar 13, 2014

pardo-bsso commented Mar 13, 2014

jmrunge commented Mar 17, 2014

pardo-bsso commented Mar 19, 2014

pardo-bsso commented Mar 19, 2014

pardo-bsso commented Mar 19, 2014

pardo-bsso commented Mar 19, 2014

jmrunge commented Mar 19, 2014

jmrunge commented Mar 19, 2014

jmrunge commented Mar 19, 2014

xaiki commented Mar 25, 2014

jmrunge commented Mar 26, 2014

pardo-bsso commented Mar 27, 2014