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

Requires SUPER permissions #12

Closed
cdwiegand opened this issue Oct 1, 2013 · 5 comments
Closed

Requires SUPER permissions #12

cdwiegand opened this issue Oct 1, 2013 · 5 comments

Comments

@cdwiegand
Copy link

Apparently this requires SUPER permission on the mysql host as line 22 of connect-mysql.js has connection.query('SET GLOBAL event_scheduler = 1'); which doesn't work for a non-SUPER user.

@nlf
Copy link
Owner

nlf commented Oct 1, 2013

This can be worked around by setting the option cleanup: false which will keep the module from attempting to schedule the event cleanup, and bypassing the need for SUPER permission. It does, however, mean that stale sessions will not be removed.

@nlf nlf closed this as completed Oct 1, 2013
@scottgonzalez
Copy link
Contributor

@nlf Would you accept a patch that changes how cleanup works so that it just does the scheduling in node? It shouldn't create a performance issue since the time spent in node will just be initiating the query, and all the work will happen outside of the event loop anyway. If you'd accept that, I'll file an issue for it and start working on a patch.

@nlf
Copy link
Owner

nlf commented Dec 19, 2013

Sure, that's a reasonable workaround. I would suggest keeping cleanup optional, however. And the user should be able to decide between having the job in mysql or having it run in node.

@scottgonzalez
Copy link
Contributor

If you want to keep the current implementation as an option, I'd suggest having it try to use the mysql job, and if that fails, check the error code. If it failed because of ER_SPECIFIC_ACCESS_DENIED_ERROR, then use the node job. Boolean cleanup option would continue to exist.

@nlf
Copy link
Owner

nlf commented Dec 19, 2013

Seems reasonable 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

No branches or pull requests

3 participants