Skip to content

change hmset to hset on is start dashboard method since hmset was dep…#11632

Closed
tahentx wants to merge 1 commit intoray-project:masterfrom
tahentx:redis_hset_update
Closed

change hmset to hset on is start dashboard method since hmset was dep…#11632
tahentx wants to merge 1 commit intoray-project:masterfrom
tahentx:redis_hset_update

Conversation

@tahentx
Copy link
Copy Markdown

@tahentx tahentx commented Oct 26, 2020

…racated in redis 4.0

Why are these changes needed?

Related issue number

Checks

  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

@edoakes
Copy link
Copy Markdown
Collaborator

edoakes commented Oct 27, 2020

Thanks @tahentx!

@mfitton
Copy link
Copy Markdown
Contributor

mfitton commented Oct 27, 2020

@tahentx thank you for your contribution! Unfortunately, it seems like there's an issue, as CI is failing with errors like:

>           redis_client.hset("webui", {"url": self._webui_url})
664E           TypeError: hset() missing 1 required positional argument: 'value'

Could you please take a look and ping me afterwards? Thanks!

@tahentx
Copy link
Copy Markdown
Author

tahentx commented Oct 27, 2020

Thanks @mfitton ! After some research, it seems like this same issue came up in this repo: rq/rq#1256 , and that an update was required with the redis server. I believe it might be the resolution here because the old function (hmset) took the same arguments in the same form as the new function (hset). And I didn't touch those. I look forward to your feedback.

@kfstorm
Copy link
Copy Markdown
Member

kfstorm commented Nov 3, 2020

Guys, FYI, there is a similar PR #11776.

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.

4 participants