Skip to content
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

Remove old Lua code #5390

Merged
merged 11 commits into from
Dec 18, 2017
Merged

Remove old Lua code #5390

merged 11 commits into from
Dec 18, 2017

Conversation

cmouse
Copy link
Contributor

@cmouse cmouse commented Jun 11, 2017

Short description

Removes all old lua code from PowerDNS auth server, and provides codesharing between auth and recursor.

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled and tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added or modified regression test(s)
  • added or modified unit test(s)

@cmouse
Copy link
Contributor Author

cmouse commented Jun 18, 2017

Currently stuck at __tostring

The LuaContext used keeps the functions available stored in global table. I have not yet been able to decipher how the mapping between object<=>function name<=>function is done, but it seems to rely on pointer to the proper member function. This makes things difficult. I am assuming that there is, somewhere, some way to find out what object:function is, and that you can actually figure out this on run-time.

It is possible to get a live pointer to the C++ object in question, but I have not yet found a way to execute ::toString only if there is such member function, or figure out how to see if there is a _tostring function mapped for the object or not.

It is probably easier to find out the latter than the first, since this information is kept at global function table stored somewhere.

@cmouse cmouse force-pushed the lua branch 3 times, most recently from 410e07a to b1b1e48 Compare June 27, 2017 07:13
@cmouse
Copy link
Contributor Author

cmouse commented Jun 27, 2017

Please merge #5468 first

@cmouse cmouse force-pushed the lua branch 3 times, most recently from 4dce79f to df0ac30 Compare June 27, 2017 13:45
@cmouse
Copy link
Contributor Author

cmouse commented Jul 7, 2017

Also pending on ahupowerdns/luawrapper#35

@cmouse
Copy link
Contributor Author

cmouse commented Aug 11, 2017

Not sure why the test fails, I cannot make it fail locally.

@cmouse
Copy link
Contributor Author

cmouse commented Aug 12, 2017

The reason the tests fail is that recursor test uses pdns auth server 4.0.4, which is obviously not going to include the lua changes done here. I am not sure how to resolve this problem, please advise.

@cmouse cmouse changed the title WIP: Remove old Lua code Remove old Lua code Sep 16, 2017
Copy link
Member

@Habbie Habbie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved with one question. This looks good, nice chunk of work. I'd like either a second reviewer, or to merge this after both 4.1.0 releases.

@@ -136,6 +136,9 @@ bool AuthLua4::axfrfilter(const ComboAddress& remote, const DNSName& zone, const


bool AuthLua4::updatePolicy(const DNSName &qname, QType qtype, const DNSName &zonename, DNSPacket *packet) {
// default decision is all goes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need a documentation update?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, it's just explicitly documented here.

@Habbie
Copy link
Member

Habbie commented Dec 18, 2017

In the "ext/luawrapper: Synchronize with upstream" commit message, can you clarify synchronize? If it is a full sync, please mention the commit you synced with. If it's an import of one patch, please mention that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants