describe module inheritance #362

mikz · 2017-05-05T15:22:58Z

closes #308

improve documentation in the example
provide whole module (blacklist.lua)
test that example module

and polish a bit balancer interface, so we can easily modify it from modules

octobot · 2017-05-05T15:46:47Z

Spell Checker found issues

examples/custom-module/README.md

Line	Typo
3	executed in each nginx phase: init, init_worker, rewrite, access
3	ed in each nginx phase: init, init_worker, rewrite, access, cont
3	n each nginx phase: init, init_worker, rewrite, access, content, lo
3	te, access, content, log, post_action, balancer, header_filter, bod
3	post_action, balancer, header_filter, body_filter etc.
3	balancer, header_filter, body_filter etc.
35	need some clever use of metatables. Here is a recommended skelet

Generated by 🚫 Danger

davidor · 2017-05-09T12:43:02Z

examples/custom-module/README.md

+-- load and initialize the parent module
+local apicast = require('apicast').new()
+
+-- this is used in User-Agent


I'm missing something. I don't see how user-agent is related to this module.

Well the used module _NAME field is going to get used in User-Agent header from APIcast connecting to backend.

davidor · 2017-05-09T12:43:15Z

examples/custom-module/README.md

+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:


typo: here's a recommended...

davidor · 2017-05-09T12:47:30Z

examples/custom-module/README.md

+  return setmetatable({}, mt)
+end
+
+function _M.log()


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().

Would a comment be enough?

mayorova · 2017-05-10T16:47:06Z

examples/custom-module/README.md

+-- 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


what is mod?

That would be instance of the module. Not really sure how to write it.

Would something like _M.new() -> _M -> apicast work for you?

OK, much clearer now :)

How about:

so in this case the inheritance works like this: local instance created with _M.new() -> _M -> apicast

Perfect. Fixed.

mayorova · 2017-05-10T16:57:15Z

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

octobot assigned mikz May 5, 2017

octobot added the T-obux label May 5, 2017

mikz added B-current in progress and removed B-current labels May 5, 2017

mikz force-pushed the custom-blacklist-module branch from 4250bc2 to 6856ee1 Compare May 5, 2017 15:29

mikz force-pushed the custom-blacklist-module branch from d3fee52 to ff7df98 Compare May 5, 2017 16:13

mikz requested a review from davidor May 5, 2017 16:13

mikz added the B-current label May 5, 2017

mikz requested a review from mayorova May 5, 2017 16:31

mikz mentioned this pull request May 9, 2017

Fix the custom module example #307

Merged

davidor reviewed May 9, 2017

View reviewed changes

mikz force-pushed the custom-blacklist-module branch 2 times, most recently from 7b6f6b3 to f0f7b13 Compare May 10, 2017 07:25

mayorova reviewed May 10, 2017

View reviewed changes

mikz and others added 4 commits May 11, 2017 08:49

[example] describe module inheritance

33f952e

with an example blacklist module [spec] test blacklist example

[balancer] allow injecting custom balancer

2f2a2e7

[luarocks] install lua-resty-iputils as dev dependency

f7b42e8

required for the blacklist custom module

[doc] grammar nazi readme fixes

6b03a73

mikz force-pushed the custom-blacklist-module branch from c2ee8b5 to 6b03a73 Compare May 11, 2017 06:50

mikz merged commit 595f51b into master May 11, 2017

mikz deleted the custom-blacklist-module branch May 11, 2017 07:16

mikz added B-current and removed B-current in progress labels May 11, 2017

mikz added in progress and removed B-current labels May 11, 2017

mikz mentioned this pull request Sep 15, 2017

Module/Plugin/Policy definition #427

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

describe module inheritance #362

describe module inheritance #362

mikz commented May 5, 2017 •

edited

Loading

octobot commented May 5, 2017 •

edited

Loading

davidor May 9, 2017

mikz May 9, 2017

davidor May 9, 2017

davidor May 9, 2017

mikz May 9, 2017

davidor May 9, 2017

mayorova May 10, 2017

mikz May 10, 2017

mikz May 11, 2017 •

edited

Loading

mayorova May 11, 2017

mikz May 11, 2017

mayorova commented May 10, 2017

describe module inheritance #362

describe module inheritance #362

Conversation

mikz commented May 5, 2017 • edited Loading

octobot commented May 5, 2017 • edited Loading

Spell Checker found issues

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikz May 11, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mayorova commented May 10, 2017

mikz commented May 5, 2017 •

edited

Loading

octobot commented May 5, 2017 •

edited

Loading

mikz May 11, 2017 •

edited

Loading