search

mozilla-services / heka

DEPRECATED: Data collection and processing made easy.
http://hekad.readthedocs.org/
Other
3.4k stars 531 forks source link

Move the grammar portion of the BIND query log decoder script into it… #1945

Closed nickchappell closed 8 years ago

nickchappell commented 8 years ago

…s own module, and make the decoder script reference it.

@trink @rafrombrc Merging #1916 after I modularized the decoder caused it to break and crash Heka on startup.

This PR keeps the modularized grammar and moves it to modules/ so it's usable elsewhere and re-adds the actual usable decoder script.

Would you like a PR against https://github.com/mozilla-services/lua_sandbox/ to add the bind.lua module there as well?

rafrombrc commented 8 years ago

Overall this looks pretty good. I've added a few comments. The only additional note I have is related to the docs. Currently you've got the docs in the comments of the Lua file, and then another copy of them in the docs folder. For the other sandbox plugins, we just pull the Lua file comments into the generated docs so we don't have to maintain two copies. You can look at the apache access decoder page to see how it works.

nickchappell commented 8 years ago

@rafrombrc I made the recommended changes. On the doc issue, I removed the redundant example usage from the block comment in the Lua script, but will Sphinx pull in all of the block comments from the Lua file or just the first block comment?

rafrombrc commented 8 years ago

The bind_query_log.rst file shouldn't contain any of the actual documentation, it should all be contained in the Lua comment. Sphinx pulls in all of the text in between two specially constructed (and explicitly specified) comment markers in the lua file. Please look carefully at the apache access log doc source and lua code to see what I mean.

nickchappell commented 8 years ago

@rafrombrc I've made the requested changes to the error message and the rst/Lua docs. Do you want me to squash all of the commits before this can be merged?

rafrombrc commented 8 years ago

Nope, this is fine. Thanks for your patience!

nickchappell commented 8 years ago

@rafrombrc would you like a PR against the https://github.com/mozilla-services/lua_sandbox repo to add this there as well?

rafrombrc commented 8 years ago

Sure. :+1: