Clarification on documentation for ngx.exec

dakanji · 2014-09-16T23:44:05+00:00

I am wanting to provide HA to my redis instance by using a single slave with master slave replication. I could write a simple failover logic in openresty/lua...

Clarification on documentation for ngx.exec

dakanji

Can I have some clarification on the documentation for ngx.redirect in the wiki? http://wiki.nginx.org/HttpLuaModule#ngx.exec

Basically it says Named locations are also supported, but query strings are ignored. I just wanted to make sure that since I handle php requests in a named location, @proxy, when I run ngx.exec, on a request such as "www.example.com/index.php?a=1,b=2", that this will be treated as if the request was simply "www.example.com/index.php" with the params igmored.

This seems like a bizarre and severe limitation for named locations and It will seem better to dump named locations for internal locations instead if this is the case.

Thanks

agentzh

Hello!

On Tue, Sep 16, 2014 at 8:44 AM, Dayo Akanji wrote:
> Basically it says Named locations are also supported, but query strings are
> ignored. I just wanted to make sure that since I handle php requests in a
> named location, @proxy, when I run ngx.exec, on a request such as
> "www.example.com/index.php?a=1,b=2", that this will be treated as if the
> request was simply "www.example.com/index.php" with the params igmored.
>

The document is confusing and wrong. I've rephrased it like this:

"Named locations are also supported (but the second args argument is
currently ignored)."

So basically you just cannot use the 2nd argument of the ngx.exec()
call to change the querystring but the existing querystring is always
inherited by the new named location request.

BTW, if you want to change the querystring then just use
ngx.req.set_uri_args() before calling ngx.exec() :)

Thanks for bringing this up :)

Regards,
-agentzh

dakanji

Thanks for the clarification.

On Sep 16, 2014, at 10:45 PM, Yichun Zhang (agentzh) <age...@gmail.com> wrote:

> Hello!
> 
> On Tue, Sep 16, 2014 at 8:44 AM, Dayo Akanji wrote:
>> Basically it says Named locations are also supported, but query strings are
>> ignored. I just wanted to make sure that since I handle php requests in a
>> named location, @proxy, when I run ngx.exec, on a request such as
>> "www.example.com/index.php?a=1,b=2", that this will be treated as if the
>> request was simply "www.example.com/index.php" with the params igmored.
>> 
> 
> The document is confusing and wrong. I've rephrased it like this:
> 
> "Named locations are also supported (but the second args argument is
> currently ignored)."
> 
> So basically you just cannot use the 2nd argument of the ngx.exec()
> call to change the querystring but the existing querystring is always
> inherited by the new named location request.
> 
> BTW, if you want to change the querystring then just use
> ngx.req.set_uri_args() before calling ngx.exec() :)
> 
> Thanks for bringing this up :)
> 
> Regards,
> -agentzh
> 

The way it works is actually as expected and I haven’t yet come across the need to change the args for something destined for a named location.
If it was possible, it would be a nice to have I suppose.

I don’t think it is necessary to use “currently” in the revision and think that simply saying it is ignored with the query string inherited from the referring location is clearer.

I will edit the wiki for clarity  … been a while since I touched it! 

Thanks again!

agentzh

Hello!

On Tue, Sep 16, 2014 at 3:07 PM, Dayo Akanji wrote:
> I don't think it is necessary to use "currently" in the revision and think that simply saying it is ignored with the query string inherited from the referring location is clearer.
>

The "currently" word there was added by intention. Because I'm going
to add support for the second argument by calling
ngx.req.set_uri_args() within ngx.exec() for named locations.

Regards,
-agentzh

dakanji

On Sep 17, 2014, at 1:07 AM, Dayo Akanji <dak...@gmail.com> wrote:

Thanks for the clarification.

On Sep 16, 2014, at 10:45 PM, Yichun Zhang (agentzh) <age...@gmail.com> wrote:

Hello!

On Tue, Sep 16, 2014 at 8:44 AM, Dayo Akanji wrote:
Basically it says Named locations are also supported, but query strings are
ignored. I just wanted to make sure that since I handle php requests in a
named location, @proxy, when I run ngx.exec, on a request such as
"www.example.com/index.php?a=1,b=2", that this will be treated as if the
request was simply "www.example.com/index.php" with the params igmored.

The document is confusing and wrong. I've rephrased it like this:

"Named locations are also supported (but the second args argument is
currently ignored)."

So basically you just cannot use the 2nd argument of the ngx.exec()
call to change the querystring but the existing querystring is always
inherited by the new named location request.

BTW, if you want to change the querystring then just use
ngx.req.set_uri_args() before calling ngx.exec() :)

Thanks for bringing this up :)

Regards,
-agentzh

The way it works is actually as expected and I haven’t yet come across the need to change the args for something destined for a named location.
If it was possible, it would be a nice to have I suppose.

I don’t think it is necessary to use “currently” in the revision and think that simply saying it is ignored with the query string inherited from the referring location is clearer.

I will edit the wiki for clarity … been a while since I touched it!

Thanks again!

While looking at this, can you also clarify the statement “that this method never returns”? It is not exactly clear what this means.

Also, the recommendation to use “return” with the method is silent on why exactly this should be the case and also, “return” does not appear anywhere in the module documentation which creates a bit of confusion.

1. Is “return" here (and other instances in the module documentation) referring to the main nginx “return" directive?

a. If so, does it work in the same way as the main directive?

2. Or is it an undocumented feature of ngx_lua?

a. Should it be therefore explained further if it is?

Thanks!

dakanji

I see.

On Sep 17, 2014, at 1:50 AM, Yichun Zhang (agentzh) <age...@gmail.com> wrote:

> Hello!
> 
> On Tue, Sep 16, 2014 at 3:07 PM, Dayo Akanji wrote:
>> I don't think it is necessary to use "currently" in the revision and think that simply saying it is ignored with the query string inherited from the referring location is clearer.
>> 
> 
> The "currently" word there was added by intention. Because I'm going
> to add support for the second argument by calling
> ngx.req.set_uri_args() within ngx.exec() for named locations.
> 
> Regards,
> -agentzh.

 I made some edits and sent some follow on questions.

I did remove the word "currently”. I would have thought simply leaving the documentation as relevant to how things work at present is better but please review and amend as needed.

Thanks!

agentzh

Hello!

On Tue, Sep 16, 2014 at 3:52 PM, Dayo Akanji wrote:
> 1. Is “return" here (and other instances in the module documentation)
> referring to the main nginx “return" directive?

It's the "return" statement in the Lua language, not nginx's "return" directive.

> 2. Or is it an undocumented feature of ngx_lua?

No. It's just a coding style recommendation.

> a. Should it be therefore explained further if it is?
>

I think the existing doc is clear enough. But I think we should use
the "return ngx.exit()" idiom in our own code samples for consistency
:)

Regards,
-agentzh

dakanji

Yes, I figured later it was probably the Lua return.

Thrown by it appearing outside a function where return is normally expected.

Maybe I have been damaged for good by PHP.

On Sep 17, 2014, at 2:53 AM, Yichun Zhang (agentzh) <age...@gmail.com> wrote:

Hello!

On Tue, Sep 16, 2014 at 3:52 PM, Dayo Akanji wrote:
1. Is “return" here (and other instances in the module documentation)
referring to the main nginx “return" directive?
It's the "return" statement in the Lua language, not nginx's "return" directive.

2. Or is it an undocumented feature of ngx_lua?
No. It's just a coding style recommendation.

a. Should it be therefore explained further if it is?

I think the existing doc is clear enough. But I think we should use
the "return ngx.exit()" idiom in our own code samples for consistency
:)

Regards,
-agentzh

The "return ngx.exit()” style does seem a lot clearer but probably could do with an expansion as to what hint is actually being given to those reading the code … and those of us trying to write the code!.

" ... to give a visual hint to others reading the code THAT XYZ IS HAPPENING"

Reason being that, I am still none the wiser what “this method never returns” actually means.

I note that under nix.redirect,it says "This method call terminates the current request's processing and never returns”.

The part on terminating is crystal clear but the never returning is not. Never returns where/what?

I am sure it is obvious to you but some of us are part timers lol!

Perhaps all instances should just simply state that the concerned methods terminate the processing and leave it at that.

Thanks!