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

Port Python 2 API to Python 3 #94

Merged
merged 1 commit into from
Dec 3, 2021
Merged

Port Python 2 API to Python 3 #94

merged 1 commit into from
Dec 3, 2021

Conversation

sjoegren
Copy link
Contributor

@sjoegren sjoegren commented Dec 1, 2021

Copy livestatus Python API from api/python to api/python3 and make
necessary changes to run under Python 3 (somewhat tested on 3.6).

The original GPL'ed sources are from commit
e06cefe.

Signed-off-by: Aksel Sjögren asjogren@itrsgroup.com

Copy link
Contributor

@jacobbaungard jacobbaungard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems fine to me. Would be nice if we could have one version that supports both python2&3 but perhaps that's not worth the effort?

@sjoegren
Copy link
Contributor Author

sjoegren commented Dec 3, 2021

Seems fine to me. Would be nice if we could have one version that supports both python2&3 but perhaps that's not worth the effort?

I messed around a bit and created one such version that works with 2/3. I'll update the PR..

Re-factor the Python 2 Livestatus library to work under both 2/3:

* Replace some py2-only idioms and fix some errors like trying to access
  `.keys()` on a list.

* Let all literal strings be unicode on both py2/3
  (`from __future__ import unicode_literals`). This way, the modules
  code internally works with unicode stings (`unicode` on py2, `str` on
  py3) and converts to byte strings when writing to socket.

* API methods take either unicode strings or byte (`str` on py2, `bytes`
  on py3) as input.

Signed-off-by: Aksel Sjögren <asjogren@itrsgroup.com>
@jacobbaungard
Copy link
Contributor

Nice, looks good!

@sjoegren sjoegren merged commit 279e618 into naemon:master Dec 3, 2021
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