Closed nickchappell closed 8 years ago
Hello @nickchappell ,
The parser should probably go to lua_sandbox, to be available to other tools.
@trink should I close this PR and open one up against https://github.com/mozilla-services/lua_sandbox/ ?
The PR against heka is actually fine for the moment it will be easier to migrate everything from one place.
@trink should the refactor to make the grammar a module wait until then as well, or should I do that now? If it should be modular-ized now, are there any examples of existing modules I should follow?
It should be modularized now. Here is an example: https://github.com/mozilla-services/lua_sandbox/blob/master/modules/ip_address.lua
Would you have to write another Lua script that calls the modular-ized BIND decoder in order to actually use the grammar?
@trink I addressed the issues you mentioned in the inline comments, minus the hostname grammar refactor, as you mentioned that would be taken care of in #129. Do you want me to squash all of the commits?
Yes please squash them
@trink @rafrombrc Commits have been squashed.
@trink anything else needed so this can be merged?
@trink any updates?
@trink @rafrombrc Any update on getting this merged?
@nickchappell Sorry for the delay, this request is in a state of limbo as it will need to be refactored and moved to the lua_sandbox in the very near future. @rafrombrc thoughts?
Hi @trink,
It should already be modular-ized. I think you mentioned earlier that this could be merged into Heka now and then added to the new sandbox at some point later.
Thanks!
The module has to be pulled out and placed in the modules directory and the decoder has to require it and implement the expected sandbox API. The question above was for Rob to determine timeline/frame and where the work should be committed (I guess he is fine with Heka, so it should be fixed there).
Yes, sorry, I misinterpreted the thread as an r+ from Trink, so I merged. Happy to have it land in Heka for now.
@rafrombrc @trink do you want this squashed into a single commit?