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

little bit of refactoring and thoughts about the future #2

Open
zzzcpan opened this issue Dec 13, 2011 · 4 comments
Open

little bit of refactoring and thoughts about the future #2

zzzcpan opened this issue Dec 13, 2011 · 4 comments

Comments

@zzzcpan
Copy link

zzzcpan commented Dec 13, 2011

Hi. Recently I was looking into your module and have couple of suggestions that may help a bit in the future (or not).
Actually, I'm more interested in merging your module with my distribution than anything else. I think it would benefit everyone.

So it would be nice to have everything restructured and put together into a single file so we can merge. As time permits of course. We can discuss this privately.

And other thing. I've noticed you aren't using internal variables for PSGI ENV. It can help a bit to make things less complex and less dependent on internals in the future. Basically you can just map every internal variable into PSGI ENV.

Here's a simple example on using variables (you probably can take it out of nginx.xs, but just in case; and don't take it as something important, it isn't):

HV  *hv;
ngx_str_t  var;
ngx_http_variable_value_t  *vv;
ngx_uint_t  hash;
...

hv = newHV (); /* PSGI ENV */
...

hash = ngx_hash_key ((u_char *) "scheme", sizeof ("scheme") - 1);

var.len  = sizeof ("scheme") - 1;
var.data = (u_char *) "scheme"; 

vv = ngx_http_get_variable(r, &var, hash);

if (vv != NULL && !vv->not_found) {

    hv_store (hv, "psgi.url_scheme", sizeof("psgi.url_scheme") - 1, 
                newSVpvn ((char *) vv->data, vv->len), 0);
}
@yko
Copy link
Owner

yko commented Dec 16, 2011

Hello Alexandr

First of all I want to thank for your interest. The same as you are interested in ngx_mod_psgi, I'm interested in Nginx::Engine and nginx-perl since they were announced. It would be great to cooperate with you. I am also interested in possibility to add support of nginx-perl to ngx_mod_psgi or vice versa.

ngx_mod_psgi was designed as third-party module for nginx and I would like to follow this idea and keep source code split up into few logical parts (files) to make maintenance easier and keep understandability.
I guess I need to review my not yet published draft impl of “Delayed Response and Streaming Body” , it is possible that there could be something which affects incorporation or support of nginx-perl.

However I'm open for discussion and new ideas. I believe we can find a solution acceptable for both sides and for happiness of existing and future users.

It would be really nice to discuss details on russian/ukrainian or even meet in person if you have plans to be somewhere around Kiev (I guess you're Ukrainian, but I don't know exactly where are you from).

And thank you for your hint, I'll review your code sample and all about nginx internal variables and I’ll see if I can use it. I always welcome anything which could simplify and speed up the code.

@zzzcpan
Copy link
Author

zzzcpan commented Dec 16, 2011

Originally I thought about using git's submodule feature to integrate
your module into nginx-perl to keep them separate.
However, I need a way to pass my interpreter instance to your module.
And this will require some work for both of us. It shouldn't be any
hard though once we decide on the interface.

Email me if you have any ideas. I just thought discussing it here may
interest some of the watchers.

@zzzcpan
Copy link
Author

zzzcpan commented Dec 28, 2011

What do you think about supporting two versions of your module?
I can strip it down and integrate into nginx-perl and all you have to do is support it :)

@yko
Copy link
Owner

yko commented Dec 28, 2011

Let's discuss this privately. Please check your email.

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

No branches or pull requests

2 participants