-
Notifications
You must be signed in to change notification settings - Fork 96
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use inotify/equivalent for detecting journal modifications (#17). #19
Conversation
Pull Request Test Coverage Report for Build 80
💛 - Coveralls |
Pull Request Test Coverage Report for Build 78
💛 - Coveralls |
This is great, thanks! Potentially it would be nice to make it handle the case of journal files not all being in the same directory by adding all directories containing journal files (and then after the journal reloads, updating the list of directories again). If you feel like writing a unit test for the reload detection, that would also be particularly awesome, though not necessary considering that a lot of existing functionality also lacks test coverage. |
…ed in via includes).
Good call. I was able to change up the file monitoring to reflect all journal file paths, even via includes, and even after updates as you suggest. Manual testing:
I'll look into adding some automated testing to this PR when I get a chance! |
Well, it's not pretty, but there's now an automated test similar to the above manual steps for the journal reload detection. Ready for re-review! |
Thanks! |
Background
See #17. The short version is that this change reduces Beancount-import's CPU usage when idle.
Changes
--debug
parameter is passed to Beancount-import. More about that below!Performance impact
Take-away: Most of the CPU consumption was actually Tornado's debug mode. Why? Well, enabling debug mode also enables autoreload, which polls all source files for changes repeatedly so as to better support the development use case of autoreloading the web server. (I confirmed it was indeed autoreload by looking at Python profiling output.)
Even though the journal change detection improvements don't have a huge impact compared to disabling the debug mode, I personally still think both are worth it. Happy to hear other opinions though!
Testing
Caveats