-
Notifications
You must be signed in to change notification settings - Fork 171
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
describe module inheritance #362
Conversation
4250bc2
to
6856ee1
Compare
Spell Checker found issuesexamples/custom-module/README.md
Generated by 🚫 Danger |
d3fee52
to
ff7df98
Compare
examples/custom-module/README.md
Outdated
-- load and initialize the parent module | ||
local apicast = require('apicast').new() | ||
|
||
-- this is used in User-Agent |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm missing something. I don't see how user-agent is related to this module.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well the used module _NAME field is going to get used in User-Agent header from APIcast connecting to backend.
examples/custom-module/README.md
Outdated
There is example module of IP blacklist in [`blacklist.lua`](blacklist.lua). | ||
|
||
To honour the module inheritance, but still be able to override some methods from the `apicast` module, you'll | ||
need some clever use of metatables. Here a recommended skeleton of the module inheritance: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: here's a recommended...
examples/custom-module/README.md
Outdated
return setmetatable({}, mt) | ||
end | ||
|
||
function _M.log() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To me, this seems to indicate that defining log()
is required, which is not the case. I would specify somewhere in this file that the purpose of this module is to log some things that apicast by itself does not, and that's why we need to override log()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would a comment be enough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes
7b6f6b3
to
f0f7b13
Compare
examples/custom-module/README.md
Outdated
-- define a table, that is going to be this module metatable | ||
-- if your table does not define a property, __index is going to get used | ||
-- and so on until there are no metatables to check | ||
-- so in this case the inheritance works like mod -> _M -> apicast |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is mod
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would be instance of the module. Not really sure how to write it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would something like _M.new() -> _M -> apicast
work for you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, much clearer now :)
How about:
so in this case the inheritance works like this: local instance created with _M.new() -> _M -> apicast
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect. Fixed.
Nice explanation! Just one thing to clarify, in a comment above... |
with an example blacklist module [spec] test blacklist example
required for the blacklist custom module
c2ee8b5
to
6b03a73
Compare
closes #308
and polish a bit balancer interface, so we can easily modify it from modules