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 deprecation warnings for msgpack > 0.5.2 #54073

Closed

Conversation

angeloudy
Copy link
Contributor

I am getting the following warnings

09:58:35,223 [WARNING ] /usr/local/lib/python3.6/site-packages/salt/transport/ipc.py:280: DeprecationWarning: encoding is deprecated, Use raw=False instead.
  self.unpacker = msgpack.Unpacker(encoding=encoding)

This commit fixed the problem

@angeloudy angeloudy requested a review from a team as a code owner July 31, 2019 00:25
@ghost ghost requested a review from cmcmarrow July 31, 2019 00:25
@waynew
Copy link
Contributor

waynew commented Jul 31, 2019

We have an existing PR that will fix the problem across the board. However, every single message from Salt master <-> minion gets piped through this transport layer, and while we have enough tests to give me a 90% confidence level that this won't break everything...

Given the potential risk, I would like to get more complete tests in place ensuring that these changes will do nothing wrong when it comes to Salt's messages.

Unfortunately, I've got some other, higher-priority items on my plate currently so I haven't been able to devote the time I would like to getting that fixed. If you're interested, more complete transport level tests would be most welcome!

@waynew
Copy link
Contributor

waynew commented Dec 5, 2019

@angeloudy I'm going to go ahead and close this PR, since it's pointing to develop, and @Akm0d I know made some changes to this recently on master. If you have any questions, please feel free to reach out!

@waynew waynew closed this Dec 5, 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.

2 participants