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

fix(conf) drop lua_code_cache option #2854

Merged
merged 1 commit into from
Sep 9, 2017
Merged

fix(conf) drop lua_code_cache option #2854

merged 1 commit into from
Sep 9, 2017

Conversation

Tieske
Copy link
Member

@Tieske Tieske commented Aug 30, 2017

This option no longer works, but triggers a lot of support
questions.

This option no longer works, but triggers a lot of support
questions.
Copy link
Member

@kikito kikito left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@thibaultcha
Copy link
Member

Before merging this, I would be very much inclined to reproduce the errors (it should be fairly easy to reproduce), and keep track of them somewhere.

@thibaultcha
Copy link
Member

Spent some time on there, and init_worker not being run when lua_code_cache is false is our issue. A lot of modules initialized in init_worker should be initialized in init instead, but cannot because of lua-resty-worker-events iirc.

Regardless, considering the implications of disabling this directive and how limiting it is (lack of ngx.semaphore, FFI contexts, and the number of lua-resty modules depending on them), it might indeed be better to simply drop support for it, I concur.

@Tieske
Copy link
Member Author

Tieske commented Sep 8, 2017

Before merging this, I would be very much inclined to reproduce the errors (it should be fairly easy to reproduce), and keep track of them somewhere.

@thibaultcha I didn't fully understand what you meant by this? considering your follow up message, it it good to merge now? if not please elaborate on what you meant by the above.

@thibaultcha
Copy link
Member

@Tieske I meant to spawn a lua_code_cache disabled instance and see how it behave. While I very much like the idea of an instance on which developers can freely edit it without having the reload Kong, I think the trouble of maintaining such a convenience isn't worth the currently needed refactor nor the potential limitations we could encounter in the future...

I say: merge!

@Tieske Tieske merged commit c7b4b48 into master Sep 9, 2017
@Tieske Tieske deleted the fix/drop-code-cache branch September 9, 2017 09:28
Tieske added a commit to Kong/docs.konghq.com that referenced this pull request Sep 11, 2017
thibaultcha pushed a commit to Kong/docs.konghq.com that referenced this pull request Oct 19, 2017
thibaultcha pushed a commit to Kong/docs.konghq.com that referenced this pull request Oct 24, 2017
thibaultcha pushed a commit to Kong/docs.konghq.com that referenced this pull request Oct 24, 2017
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