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 crash on unconnectable MySQL server (resolves #33582) #33615

Merged
merged 2 commits into from
May 31, 2016

Conversation

danslimmon
Copy link
Contributor

What does this PR do?

Fixes a stacktrace-inducing crash when the mysql module was used against a server to which it couldn't connect. Several of the functions in the mysql module didn't check whether the connection was successful before attempting to grab a cursor. Now they do.

What issues does this PR fix or reference?

#33582

Previous Behavior

$ sudo ./salt/scripts/salt saltdev mysql.get_slave_status
saltdev:
    The minion function caused an exception: Traceback (most recent call last):
      File "/home/ubuntu/salt/salt/minion.py", line 1078, in _thread_return
        return_data = func(*args, **kwargs)
      File "/home/ubuntu/salt/salt/modules/mysql.py", line 1959, in get_slave_status
        rtnv = __do_query_into_hash(conn, "SHOW SLAVE STATUS")
      File "/home/ubuntu/salt/salt/modules/mysql.py", line 1837, in __do_query_into_hash
        cursor = conn.cursor()
    AttributeError: 'NoneType' object has no attribute 'cursor'

New Behavior

$ sudo ./salt/scripts/salt saltdev mysql.get_slave_status
saltdev:

It now returns an empty array, just like the other module functions do. The states, for their part, detect the error from __context__ and fail if there was a problem with the connection.

Tests written?

Yup: unit.modules.mysql_test.MySQLTestCase.test_get_slave_status_bad_server

@danslimmon danslimmon changed the title Mysql traceback 33582 Fix crash on unconnectable MySQL server May 30, 2016
@danslimmon danslimmon changed the title Fix crash on unconnectable MySQL server Fix crash on unconnectable MySQL server (resolves #33582) May 30, 2016
@danslimmon
Copy link
Contributor Author

Hard to tell whether the broken tests here are my fault. Some are apt mutex errors, others not. What do you think?

@cachedout
Copy link
Contributor

This looks good to me. Thanks, @danslimmon

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.

2 participants