Add queue based observer #471
Conversation
When cooperating with the queue observer
|
Hey! I don't understand what the purpose of this new MongoObserver variant is. Could you elaborate on your motivation? |
|
The purpose is to fix issues like #434. When the internet connection is disrupted, events can just accumulate in the queue. When the connection is restored, all events will be written to the database as originally intended. I tried to reflect this in the tests as well. Moreover, I expect some performance improvements, because IO should now happen in a separate thread. |
|
Hi, I guess this is relevant to all observers as they all could be remote servers depending on a network connection. I would be up for it if all observers read from a queue. As a suggestion, I think the go-to tool to implement such a pattern would be pyzmq. |
|
@johny-c yes, with the general QueueObserver class you can wrap any observer and make it use a queue. I just made it a bit more convenient for the MongoObserver and also had to make some changes, because it would silently gobble up some errors. I haven't used zmq yet, but it seems to be a bit overkill as the current implementations works quite well and the observers mostly do IO anyway. |
|
Nice, didn't see that. This seems like a major feature. Perhaps this should be indicated in a new sacred version (release)? |
|
yeah, documentation is missing I guess. I would like to try this out "in production" under the radar before releasing. Probably needs extra tests for each observer then as well to make sure they do not have similar problems as the MongoObserver. I will merge for now and see if unexpected problems pop up. |
|
I was waiting for this feature for a long time! Couldn't that be a default? Currently, I set up my mongodb observer using the "-m" flag. To use that, I would need to hardcore mongodb options in the code. Or maybe there is a workaround? |
|
I think in the long run this should become a default. However, I first wanted to if the approach has any unexpected problems. I could add an optional switch for the queue based variant for now. |
|
A quick feedback: works fine for me although I did not test with a broken connection. The only inconvenience is that when breaking the experiment from cmd with ctrl+c you need to press it 3 or 4 times to stop all threads I suppose. |
* 'master' of https://github.com/IDSIA/sacred: Release 0.7.5 update dependencies to safer version of SQLAlchemy fixed numpy deprecation warning fix yaml deprecation warning Make config read only (IDSIA#472) Remove broken codacy badge (IDSIA#486) Add queue based observer (IDSIA#471) Run CI against python 36 and 37 (IDSIA#485) Make failed mongo observer dump configurable (IDSIA#462) Make stale time longer (IDSIA#484) Fix a bug where a unicode char in README.rst would fail install (IDSIA#481)
…d into run_kwargs_fix * 'run_kwargs_fix' of github.com-rueberger:rueberger/sacred: Release 0.7.5 update dependencies to safer version of SQLAlchemy fixed numpy deprecation warning fix yaml deprecation warning Make config read only (IDSIA#472) Remove broken codacy badge (IDSIA#486) Add queue based observer (IDSIA#471) Run CI against python 36 and 37 (IDSIA#485) Make failed mongo observer dump configurable (IDSIA#462) Make stale time longer (IDSIA#484) Fix a bug where a unicode char in README.rst would fail install (IDSIA#481)
No description provided.