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

Python 3 compatibility - bytes and strings #47

Closed
davidc opened this issue Jul 10, 2019 · 4 comments · Fixed by #50
Closed

Python 3 compatibility - bytes and strings #47

davidc opened this issue Jul 10, 2019 · 4 comments · Fixed by #50

Comments

@davidc
Copy link

davidc commented Jul 10, 2019

  File "/home/david/mtping/RouterOS-api/routeros_api/sentence.py", line 59, in get_api_format
    formated.append(b'=' + key + b'=' + value)
TypeError: can't concat bytes to str

The fix proposed #46 does not fix this since sometimes the value is already bytes, so it causes another error this time in get_api():

Traceback (most recent call last):
  File "./mtping.py", line 55, in <module>
    api = connection.get_api()
  File "/home/david/mtping/RouterOS-api/routeros_api/api.py", line 51, in get_api
    self.api.login(self.username, self.password, self.plaintext_login)
  File "/home/david/mtping/RouterOS-api/routeros_api/api.py", line 93, in login
    'login', {'name': login.encode(), 'response': hashed})
  File "/home/david/mtping/RouterOS-api/routeros_api/resource.py", line 39, in call
    additional_queries=additional_queries).get()
  File "/home/david/mtping/RouterOS-api/routeros_api/resource.py", line 45, in call_async
    additional_queries=additional_queries)
  File "/home/david/mtping/RouterOS-api/routeros_api/api_communicator/encoding_decorator.py", line 12, in call
    path, command, arguments, queries, additional_queries)
  File "/home/david/mtping/RouterOS-api/routeros_api/api_communicator/async_decorator.py", line 6, in call
    tag = self.inner.send(*args, **kwargs)
  File "/home/david/mtping/RouterOS-api/routeros_api/api_communicator/exception_decorator.py", line 11, in send
    return self.inner.send(*args, **kwargs)
  File "/home/david/mtping/RouterOS-api/routeros_api/api_communicator/key_cleaner_decorator.py", line 11, in send
    queries=encoded_queries, additional_queries=additional_queries)
  File "/home/david/mtping/RouterOS-api/routeros_api/api_communicator/base.py", line 19, in send
    self.send_command(command)
  File "/home/david/mtping/RouterOS-api/routeros_api/api_communicator/base.py", line 37, in send_command
    self.base.send_sentence(command.get_api_format())
  File "/home/david/mtping/RouterOS-api/routeros_api/sentence.py", line 59, in get_api_format
    formated.append(b'=' + key + b'=' + str.encode(value))
TypeError: descriptor 'encode' requires a 'str' object but received a 'bytes'

It seems that sometimes the value is a string and sometimes it is already encoded into bytes.

Ideally we would store it only as one or the other, but it is necessary to allow bytes so that the hashed password in RouterOsApi.login() can still be transmitted (it starts with a null byte)

In the absence of a larger refactor, I have amended get_api_format, if the value is not already bytes, to first coerce it to string (allowing the user finally to set integer values), and then into bytes.

@davidc
Copy link
Author

davidc commented Jul 10, 2019

Please can we get this into pip ASAP as it is a blocker for us. Thanks.

@okazdal
Copy link

okazdal commented Aug 9, 2019

Fixed version forked here https://github.com/okazdal/RouterOS-api
you can clone and install with python setup.py build and python setup.py install

@davidc
Copy link
Author

davidc commented Aug 9, 2019

I mean I don't see why that was necessary given you've literally just applied my pull request to another branch, leaving your branch identical to the one I already sent a pull request from....

This needs merging into master and then into pip.

@stevehaskew
Copy link
Contributor

@davidc Sorry that I can't help you get this merged - the wheels turn pretty slowly on this project!

@jgoclawski, could you take a look at the fix PR #48 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants