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 Redis Test #5302

Merged
merged 2 commits into from
Jul 30, 2019
Merged

Conversation

simon-mo
Copy link
Contributor

Catch a different error in redis init test. Our current master are failing because of this.

____________________ TestRedisPassword.test_redis_password _____________________
self = <ray.tests.test_ray_init.TestRedisPassword object at 0x7f4e0bc35518>
password = '657c7eab2a0fbe90ec43fbb0cfe32351c37468c5a2b57113cfbd706eb95f9ff57247b38b09bb6330ac8d318059d535ba700b621c26216e2b9aff0...e4aaec9a148cc4022b85f94ca2f98e05fcbfe62f3713d804a911131aa0db606c16891a95ac5a6964ad30d342cef7e7da6653c8d706cfb5812f0f72'
shutdown_only = None
    @pytest.mark.skipif(
        os.environ.get("RAY_USE_NEW_GCS") == "on",
        reason="New GCS API doesn't support Redis authentication yet.")
    def test_redis_password(self, password, shutdown_only):
        @ray.remote
        def f():
            return 1
    
        info = ray.init(redis_password=password)
        redis_address = info["redis_address"]
        redis_ip, redis_port = redis_address.split(":")
    
        # Check that we can run a task
        object_id = f.remote()
        ray.get(object_id)
    
        # Check that Redis connections require a password
        redis_client = redis.StrictRedis(
            host=redis_ip, port=redis_port, password=None)
        with pytest.raises(redis.ResponseError):
>           redis_client.ping()
python/ray/tests/test_ray_init.py:42: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
../../../.local/lib/python3.6/site-packages/redis-3.3.0-py3.6.egg/redis/client.py:1106: in ping
    return self.execute_command('PING')
../../../.local/lib/python3.6/site-packages/redis-3.3.0-py3.6.egg/redis/client.py:839: in execute_command
    return self.parse_response(conn, command_name, **options)
../../../.local/lib/python3.6/site-packages/redis-3.3.0-py3.6.egg/redis/client.py:853: in parse_response
    response = connection.read_response()
../../../.local/lib/python3.6/site-packages/redis-3.3.0-py3.6.egg/redis/connection.py:671: in read_response
    response = self._parser.read_response()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
self = <redis.connection.PythonParser object at 0x7f4ddca88cf8>
    def read_response(self):
        response = self._buffer.readline()
        if not response:
            raise ConnectionError(SERVER_CLOSED_CONNECTION_ERROR)
    
        byte, response = byte_to_chr(response[0]), response[1:]
    
        if byte not in ('-', '+', ':', '$', '*'):
            raise InvalidResponse("Protocol Error: %s, %s" %
                                  (str(byte), str(response)))
    
        # server returned an error
        if byte == '-':
            response = nativestr(response)
            error = self.parse_error(response)
            # if the error is a ConnectionError, raise immediately so the user
            # is notified
            if isinstance(error, ConnectionError):
>               raise error
E               redis.exceptions.AuthenticationError: Authentication required.


@simon-mo simon-mo mentioned this pull request Jul 28, 2019
@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/15733/
Test PASSed.

@simon-mo simon-mo requested a review from robertnishihara July 29, 2019 21:04
@robertnishihara
Copy link
Collaborator

The change was probably introduced in the last Redis release, which happened today https://pypi.org/project/redis/3.3.2/#history

This fix probably won't work with older versions of the Python Redis module. So maybe we should pin the Redis version to >=3.3.2?

@simon-mo
Copy link
Contributor Author

@robertnishihara pinned!

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/15777/
Test PASSed.

@robertnishihara robertnishihara merged commit 196495a into ray-project:master Jul 30, 2019
edoakes pushed a commit to edoakes/ray that referenced this pull request Aug 9, 2019
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