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

build ngx kafka module as dynamic module #7

Merged
merged 2 commits into from
Sep 12, 2016

Conversation

yourpleasure
Copy link
Contributor

I change the config file and now the module can be compiled as both static and dynamic module.

@salekseev
Copy link

+1

@brg-liuwei
Copy link
Owner

If you compile module as a dynamic module, you must add

    load_module /path/to/ngx_http_kafka_module.so;

before "http" directive besides adding the code to nginx config file. After that you can use the module by just executing `nginx -c /path/to/nginx.conf -s reload`.

The position of directive load_module maybe have some mistakes. I tried the conf as follows with nginx -t and the error occurs: nginx: [emerg] "load_module" directive is specified too late in /opt/mynginx/conf/nginx.conf

events {
    worker_connections  1024;
}

load_module /opt/mynginx/modules/ngx_http_kafka_module.so;

http {
    include       mime.types;
    default_type  application/octet-stream;

Can you describe more precise for the position of directive load_module in README.md?

@yourpleasure
Copy link
Contributor Author

Sorry. This is a mistake. We can put

load_module /path/to/ngx_http_kafka_module.so;

at the very beginning of the nginx config file. In my test environment, I put it before 'event' directive and it works. I think it may work when I put it just before 'http' directive, but it fails. I have changed the README file.

@yourpleasure
Copy link
Contributor Author

Sorry again. I don't observe that your repo has been updated. I will merge it and do some test.

@brg-liuwei
Copy link
Owner

@yourpleasure Hi, my friend, thank you very much for this PR. I tested the PR and it is worked in my environment. I'm very glad to accept this PR.

And there is an another request to you. To ensure the high quality of this module, I enabled travis-ci for auto continuous integration testing. We can edit file .travis.yml to add some scripts for auto-testing. And, do you mind contributing another PR that add some scripts in .travis.yml and add some test cases in directory t/ for auto-testing the directive of load_module /path/to/ngx_http_kafka_module.so;

@brg-liuwei brg-liuwei merged commit cd133b7 into brg-liuwei:master Sep 12, 2016
@yourpleasure
Copy link
Contributor Author

@brg-liuwei OK. I will study travis-ci and do some test.

@brg-liuwei
Copy link
Owner

@yourpleasure Travis-ci is easy to study and is very interesting. For addition, I strongly recommend you to learn about Test::Nginx. You can use it to create many test case very easily. Wish you can like those tools, and if this is any problems, feel free to send mail to me :).

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

Successfully merging this pull request may close these issues.

3 participants