Python 3 support

[geertj]

It would be good if at least parts of txdbus would work on Python 3.x. It makes integrating txdbus into my "gruvi" project easier. And I'd expect it'd be useful for the asyncio folks as well (And hopefully at some point Twisted will support Python 3 as well.)

I'm happy to contribute patches. However, the best way to support both Python 2.x and 3.x (which is generally accepted now in the Python community), is to have a single-source project that supports 2.6, 2.7 and 3.3+ (3.2 is possible as well but 3.3+ is easier).

So the question is... are you currently supporting Python 2.5 and if so are you willing to give up on that?

[cocagne]

I've wanted to start working on Python 3 support for a while now. Recent versions of Twisted include sufficient py3k support for txdbus but I simply haven't been able to find the necessary bandwidth. Those patches will go a long way towards making that happen. Thanks!

As to the portability issue, I fully agree with supporting only 2.7/3.3+ going forward. The existing version is pretty stable but occasional bug reports do trickle in. I think I'll leave the master branch on the 1.0 version and work on the py3ksupport in a separate branch. Though, now that I think about it, that's probably backwards. Meh, I haven't had my morning coffee yet.

I've given you full collaborator access to the txdbus repository. You have free reign to make whatever changes you see fit. All I ask is that you keep it clear where the old, stable code is. Either in the mainline with py3k on a separate branch or vice versa.

Thanks again for the patches :)

Tom

On Sat, May 10, 2014 at 10:19 AM, Geert Jansen notifications@github.comwrote:

It would be good if at least parts of txdbus would work on Python 3.x. It makes integrating txdbus into my "gruvi" project easier. And I'd expect it'd be useful for the asyncio folks as well (And hopefully at some point Twisted will support Python 3 as well.)

I'm happy to contribute patches. However, the best way to support both Python 2.x and 3.x (which is generally accepted now in the Python community), is to have a single-source project that supports 2.6, 2.7 and 3.3+ (3.2 is possible as well but 3.3+ is easier).

So the question is... are you currently supporting Python 2.5 and if so are you willing to give up on that?

— Reply to this email directly or view it on GitHubhttps://github.com/cocagne/txdbus/issues/11 .

[geertj]

Thanks Tom!

I'm happy to do some more work on Py3K support in txdbus. I would offer the following thoughts for your consideration:

• In my view, a single active branch that supports both Python 2.x and 3.x is much preferable over a stable/frozen branch for 2.x and a new-feature branch for 3.x. Maintaining two branches is a lot of work, and there's a big potential for error when backporting the inevitable fixes. On the other hand, there's hardly any drawback writing code that runs on both 2.x and 3.x. The fact that you can't use any new Python 3.x features in my experience is not an issue.
• Supporting Python 2.6 is easy. There's tiny differences between 2.6 and 2.7 but they are easy to work around. The only thing that pops to mind here is implicit numbering for positional arguments for '{}' format strings. I'm sure there's more but I haven't bumped into anything else. The benefit of supporting 2.6 is that the current stable version of the Enterprise Linux distro's (RHEL 6, CentOS 6, ...) are still on 2.6.
• A more difficult question is whether to support 2.5 as well. From reading the docs, it seems Twisted (12.x) still supports it. Adding support for 2.5 is not impossible, but cumbersome. If possible, it would be easier not to support it. Do you need 2.5, or are you aware of other txdbus users that do? For reference, 2.5 was released 8 years ago in 2006. I think the only users of it would be RHEL 5 and its clones, and potentially some embedded system. I doubt any of these need D-BUS.

So, in summary, my recommendation would be to go for a single branch that supports 2.6, 2.7 and 3.3+. There's not a whole lot of extra effort in supporting these platforms, and the result is pretty decent coverage of the current Python versions out in the wild. And using the "tox" driver it is also very easy to test for all of these.

Let me know your what you think..

Regards, Geert

[cocagne]

That sounds reasonable. However, I do think we should at least fork off a maintenance branch for the 1.0 version. It may not see any use but I'd still like to be able to fix bugs for people stuck on old releases (python 2.5, twisted 12, etc). Keeping all future development on a unified codebase sounds like a good idea though.

As for the versioning, I don't see any need for supporting Python 2.5. Recent versions of Twisted now require 2.6/3.3+ so we might as well follow suit. Along those lines, we ought to bump the version in the mainline to 1.1 due to dropping support for the older versions.

Tom

On Tue, May 13, 2014 at 2:01 AM, Geert Jansen notifications@github.comwrote:

Thanks Tom!

I'm happy to do some more work on Py3K support in txdbus. I would offer the following two thoughts for your consideration:

-

In my view, a single active branch that supports both Python 2.x and 3.x is much preferable over a stable/frozen branch for 2.x and a new-feature branch for 3.x. Maintaining two branches is a lot of work, and there's a big potential for error when backporting the inevitable fixes. On the other hand, there's hardly any drawback writing code that runs on both 2.x and 3.x. The fact that you can't use any new Python 3.x features in my experience is not an issue.

Supporting Python 2.6 is easy. There's tiny differences between 2.6 and 2.7 but they are easy to work around. The only thing that pops to mind here is implicit numbering for positional arguments for '{}' format strings. I'm sure there's more but I haven't bumped into anything else. The benefit of supporting 2.6 is that the current stable version of the Enterprise Linux distro's (RHEL 6, CentOS 6, ...) are still on 2.6.

A more difficult question is whether to support 2.5 as well. From reading the docs, it seems Twisted (12.x) still supports it. Adding support for 2.5 is not impossible, but cumbersome. If possible, it would be easier not to support it. Do you need 2.5, or are you aware of other txdbus users that do? For reference, 2.5 was released 8 years ago in 2006. I think the only users of it would be RHEL 5 and its clones, and potentially some embedded system. I doubt any of these need D-BUS.

So, in summary, my recommendation would be to go for a single branch that supports 2.6, 2.7 and 3.3+. There's not a whole lot of extra effort in supporting these platforms, and the result is pretty decent coverage of the current Python versions out in the wild. And using the "tox" driver it is also very easy to test for all of these.

Let me know your what you think..

Regards, Geert

— Reply to this email directly or view it on GitHubhttps://github.com/cocagne/txdbus/issues/11#issuecomment-42923364 .

[geertj]

OK, understood. So in summary:

• Create a new empty branch from the current master.
• Merge the py3ksupport branch into master.
• Don't worry about 2.5 support in the master.

If that's allright, I can do it probably tomorrow.

Also I have some smaller patches not related to Python 3 support. How would you prefer to handle these? Shall I create pull requests and you merge them? Or at least ACK them?

[cocagne]

Yeah, that about sums it up. As for the patches, just make sure they're fully covered by passing unit tests before you merge them. If you're thinking about doing any of the obviously major stuff like breaking backwards compatibility or adding new dependencies, I'd appreciate it if we could discuss it first. For the rest of it though just go for it. I've looked through gruvi's codebase and it's obvious you know what you're doing.

Tom

On Wed, May 14, 2014 at 6:54 AM, Geert Jansen notifications@github.comwrote:

OK, understood. So in summary:

• Create a new empty branch from the current master.
• Merge the py3ksupport branch into master.
• Don't worry about 2.5 support in the master.

If that's allright, I can do it probably tomorrow.

Also I have some smaller patches not related to Python 3 support. How would you prefer to handle these? Shall I create pull requests and you merge them? Or at least ACK them?

— Reply to this email directly or view it on GitHubhttps://github.com/cocagne/txdbus/issues/11#issuecomment-43071175 .

[geertj]

OK - I created the maintenance branch (txdbus-10) and I merged py3support into the master!

I have some more Py3K fixes for authentication.py. I'll push those in the next few days.

We should try out the pre-release version of Twisted for Py3K and get the whole of txdbus to run on it. From my side I'm a little short on time in the next couple of months so I'm not sure if and how fast I'd get to it

Going forward, any changes that I'm not 100% certain about I'll create a PR and ask you to review.

Geert

[wolfhechel]

I've been meaning to port this library to asyncio, since it's already written for Twisted I imagine rewriting it wouldn't be to much hassle.

Would it be interesting however to keep both the twisted and asyncio codebase in this project or would you prefer a fork?

[cocagne]

Interesting idea. I haven't played with asyncio yet but I've heard good things. With respect to your question though, it'd probably be a good idea to stick to a single code base. KDBus is likely to be merged into the kernel soon and it will require some pretty big changes to support. I suspect that doing it just once would be easier than porting the changes over to a fork.

Tom

On Wed, May 20, 2015 at 1:09 AM, Pontus Karlsson notifications@github.com wrote:

I've been meaning to port this library to asyncio, since it's already written for Twisted I imagine rewriting it wouldn't be to much hassle.

Would it be interesting however to keep both the twisted and asyncio codebase in this project or would you prefer a fork?

— Reply to this email directly or view it on GitHub https://github.com/cocagne/txdbus/issues/11#issuecomment-103775045.

[wolfhechel]

A request to build an asyncio-based D-bus protocol implementation has been approved on GSoC 2015 (request submitted by qtile-dev team). I've mentioned this idea in in a thread on the mailinglist and hopefully it will be approved, I intend to contribute to that development unless I'm prohibited to do so (not sure how GSoC works).

In my reply on that thread I've also outlined what works needs to be done to fully abstract the D-bus implementation in txdbus, in order to separate the twisted/asyncio layers on a single codebase.

[ntninja]

@wolfhechel Did anything come out of the GSoC project?

[wolfhechel]

Not much unfortunately, the development pretty much halted after the planing phase. The project I was going to use it for has been stalled so my personal interest in it is gone, I might pick up on it some time in the future once I get some time on my hands to continue my project.

[ntninja]

Ok, thanks for the update!

[bochecha]

I just sent a pull request that fixes the tests on Python 3 (#34).

I haven't played much with txdbus yet, but I'm looking into it for a project that uses Python 3.

Anybody knows what the current status is with txdbus and Python 3? Is it supposed to be working now? Or should I just try it out, and fix issues as I find them?

[cocagne]

Hi Mathieu. You'll probably need to just give it a shot and see what happens. Txdbus reached maturity before Twisted had any formal Python 3 support and, since I haven't yet had any reason to use it with Python 3, I've been relying on the community to send patches in that regard. Let me know what you find. The unit test coverage is pretty good so if the tests are passing, I would expect there to be few major problems.

Tom

On Sun, May 29, 2016 at 7:01 AM, Mathieu Bridon notifications@github.com wrote:

I just sent a pull request that fixes the tests on Python 3 (#34 https://github.com/cocagne/txdbus/pull/34).

I haven't played much with txdbus yet, but I'm looking into it for a project that uses Python 3.

Anybody knows what the current status is with txdbus and Python 3? Is it supposed to be working now? Or should I just try it out, and fix issues as I find them?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/cocagne/txdbus/issues/11#issuecomment-222356974, or mute the thread https://github.com/notifications/unsubscribe/ABrLfHv8XjVYMW1-eEOQfHCO-qRuFrrhks5qGX__gaJpZM4B5xdc .

[WhyNotHugo]

In case anybody following this thread is still following up and interested, you might want to look at #51 (feedback appreciated).

[WhyNotHugo]

The only tests not yet working on python3 are currently txdbus.test.test_authentication.ServerObjectTester.*. All others pass and are being run via CI.

Please note that tests are not run via trial.

[oberstet]

@hobarrera I am most definitely interested in Python 3 support!

Are you running coverage reports on the CI?

Also, this is making me slightly nervous:

(cpy361_7) oberstet@thinkpad-t430s:~/scm/3rdparty/txdbus$ find . -name "*.py" -exec grep -H "str(" {} \;
./txdbus/marshal.py:        raise MarshallingError('Invalid interface name "%s": %s' % (n, str(e)))
./txdbus/marshal.py:        raise MarshallingError( str(e).replace( 'interface', 'error', 1 ) )
./txdbus/marshal.py:        raise MarshallingError('Invalid bus name "%s": %s' % (n, str(e)))
./txdbus/marshal.py:        raise MarshallingError('Invalid member name "%s": %s' % (n, str(e)))
./txdbus/client.py:                    msg = 'Expected "%s". Received "%s"' % (str(returnSignature), str(msg.signature))
./txdbus/message.py:#                print '    %s = %s' % (a.ljust(15), str(getattr(self,a)))
./txdbus/message.py:        raise error.MarshallingError('Unknown Message Type: ' + str(messageType))
./txdbus/bus.py:            raise DError('org.freedesktop.DBus.Error.InvalidArgs', str(e))
./txdbus/protocol.py:                        log.msg('DBus Authentication failed: ' + str(e))
./txdbus/test/client_tests.py:                              str(e.value))
./txdbus/test/client_tests.py:                              str(e.value))
./txdbus/test/client_tests.py:                              str(e.value))
./txdbus/test/client_tests.py:            self.assertEquals(str(e.value),
./txdbus/test/client_tests.py:            self.assertEquals( str(e.value),
./txdbus/test/client_tests.py:            self.assertEquals( str(e.value),
./txdbus/test/client_tests.py:            self.assertEquals( str(e.value),
./txdbus/test/client_tests.py:            self.assertEquals( str(e.value),
./txdbus/test/client_tests.py:            print(str(e.value))
./txdbus/test/client_tests.py:            print(str(e.value.message))
./txdbus/test/client_tests.py:                self.assertEquals(str(e), 'Signal "InvalidSignalName" not found in any supported interface.')        
./txdbus/test/client_tests.py:                              str(e.value))
./txdbus/test/client_tests.py:                              str(e.value))
./txdbus/test/client_tests.py:                              str(e.value))
./txdbus/test/client_tests.py:                              str(e.value))
./txdbus/test/test_validators.py:        self.t('.'.join([ 'a' + str(i) for i in range(0,200) ]))
./txdbus/test/test_validators.py:        self.t('.'.join([ 'a' + str(i) for i in range(0,200) ]))
./txdbus/test/test_endpoints.py:            self.assertEquals(str(e), 'DBus Session environment variable not set')
./txdbus/test/test_message.py:            self.assertEquals(str(e), 'Unknown Message Type: 99')
./txdbus/authentication.py:                log.msg('DBUS Cookie authentication failed: ' + str(e))
./txdbus/authentication.py:                self.sendAuthMessage(b'ERROR ' + str(e).encode('unicode-escape'))
./txdbus/authentication.py:    cookieContext = 'org_twisteddbus_ctx' + str(os.getpid())
./txdbus/authentication.py:                         str(self.cookieId).encode('ascii'),
./txdbus/authentication.py:        cookies.append((str(cookie_id).encode('ascii'), str(int(timefunc())).encode('ascii'), cookie) )
./txdbus/endpoints.py:                path = d['tmpdir'] + '/dbus-' + str(os.getpid())
./txdbus/introspection.py:        iname = str(attrs['name'])
./txdbus/introspection.py:        self.member       = interface.Method(str(attrs['name']))
./txdbus/introspection.py:        self.member       = interface.Signal(str(attrs['name']))
./txdbus/introspection.py:        name      = str(attrs['name'])
./txdbus/introspection.py:        sig       = str(attrs['type'])
./txdbus/introspection.py:        rw        = str(attrs['access'])
./txdbus/introspection.py:            self.member.emits = str(attrs['value']) in ('true','invalidates')
./txdbus/introspection.py:        t = str(attrs['type'])
(cpy361_7) oberstet@thinkpad-t430s:~/scm/3rdparty/txdbus$ find . -name "*.py" -exec grep -H "u'" {} \;
./txdbus/marshal.py:               ('UINT32',      'u',     4),
./txdbus/marshal.py:    dbusSignature = 'u'
./txdbus/marshal.py:                    'u' : UInt32,
./txdbus/marshal.py:                'u' : marshal_uint32,
./txdbus/marshal.py:                  'u' : unmarshal_uint32,
./txdbus/client.py:                            signature   = 'su',
./txdbus/bus.py:               Method('RequestName',           arguments='su', returns='u' ),
./txdbus/bus.py:               Method('ReleaseName',           arguments='s',  returns='u' ),
./txdbus/bus.py:               Method('GetConnectionUnixUser', arguments='s',  returns='u' ),
./txdbus/bus.py:               Method('GetConnectionUnixProcessId' , arguments='s',     returns='u' ),
./txdbus/bus.py:               Method('StartServiceByName',          arguments='su',    returns='u'),
./txdbus/bus.py:               Method('GetAdtAuditSessionData',      arguments='s',     returns='u'),
./txdbus/bus.py:               Method('GetConnectionSELinuxSecurityContext', arguments='su', returns='ay'),
./txdbus/test/client_tests.py:            expected = repr(u'foo') + ' # ' + repr([1, 2, [u'substring', 10], 4])
./txdbus/test/test_marshal.py:#               ('UINT32',      'u',     4),
./txdbus/test/test_marshal.py:        self.check( 'u', 70000, pack('I', 70000))
./txdbus/test/test_marshal.py:        self.check( 'u', 70000, pack('I', 70000))
(cpy361_7) oberstet@thinkpad-t430s:~/scm/3rdparty/txdbus$ find . -name "*.py" -exec grep -H 'u"' {} \;
(cpy361_7) oberstet@thinkpad-t430s:~/scm/3rdparty/txdbus$ find . -name "*.py" -exec grep -H "six." {} \;
./setup.py:    install_requires = ['twisted>=10.1', 'six'],
./txdbus/marshal.py:    elif isinstance(pobj, six.integer_types): return 'x'
./txdbus/marshal.py:    elif isinstance(pobj, six.string_types): return 's'
./txdbus/marshal.py:        for k,v in six.iteritems(pobj):
./txdbus/marshal.py:            ct = six.next(g)
./txdbus/marshal.py:    if not isinstance(var, six.string_types):
./txdbus/marshal.py:        arr_list = [ tpl for tpl in six.iteritems(var) ]
./txdbus/client.py:                if isinstance(merr.body[0], six.string_types):
./txdbus/router.py:        for r in six.itervalues(self._rules):
./txdbus/test/test_authentication.py:        if six.PY3:
./txdbus/test/test_client_against_native_bus.py:    for k,v in six.iteritems(client_tests.__dict__):
./txdbus/test/test_marshal.py:        if six.PY2:
./txdbus/test/test_marshal.py:        self.t(bytearray(six.b('\xAA\xAA')), 'ay')
./txdbus/test/test_marshal.py:        self.check('ay', bytearray(six.b('\xaa\xaa')), pack('iBB', 2, 170, 170))
./txdbus/test/test_marshal.py:        self.check('v', bytearray(six.b('\xAA\xAA')), pack('B2siBB', 2, six.b('ay'), 2, 170, 170))
./txdbus/objects.py:                for name, obj in six.iteritems(base.__dict__):
./txdbus/objects.py:                for ic in six.itervalues(cache):
./txdbus/objects.py:                    for p in six.itervalues(ifc.properties):
./txdbus/objects.py:                for ifc in six.itervalues(cache):
./txdbus/objects.py:                    for p in six.itervalues(ifc.properties):
./txdbus/authentication.py:        for n, m in six.iteritems(self.authenticators):
./txdbus/interface.py:            k = list(six.iterkeys(self.methods))
./txdbus/interface.py:            k = list(six.iterkeys(self.signals))
./txdbus/interface.py:            k = list(six.iterkeys(self.properties))
./txdbus/introspection.py:from six.moves import cStringIO

[WhyNotHugo]

Nope, but we should add coverage. Most of those str usages should be fine and/or are actually part of the py3 upgrade.

Also, I don't see why you're pointing out the six usages there. (not sure what why that makes you nervous).

[oberstet]

rgd coverage, could be stolen from https://github.com/crossbario/txaio#txaio https://codecov.io/github/crossbario/txaio

rgd six, I was just puzzled not finding any use of six.binary_type, six.text_type or six.integer_types, which usually are needed for type checking / assert ..

another thing that would be nice: flake8 / pep8 compliance.

eg flake8 --statistics .:

1     E111 indentation is not a multiple of four
2     E114 indentation is not a multiple of four (comment)
4     E115 expected an indented block (comment)
3     E116 unexpected indentation (comment)
3     E124 closing bracket does not match visual indentation
2     E125 continuation line with same indent as next logical line
25    E127 continuation line over-indented for visual indent
10    E128 continuation line under-indented for visual indent
666   E201 whitespace after '('
640   E202 whitespace before ')'
100   E203 whitespace before ':'
319   E221 multiple spaces before operator
1     E222 multiple spaces after operator
5     E225 missing whitespace around operator
3     E228 missing whitespace around modulo operator
166   E231 missing whitespace after ','
372   E251 unexpected spaces around keyword / parameter equals
30    E261 at least two spaces before inline comment
38    E265 block comment should start with '# '
6     E271 multiple spaces after keyword
3     E272 multiple spaces before keyword
8     E301 expected 1 blank line, found 0
45    E302 expected 2 blank lines, found 1
323   E303 too many blank lines (2)
4     E305 expected 2 blank lines after class or function definition, found 1
13    E306 expected 1 blank line before a nested definition, found 0
325   E501 line too long (92 > 79 characters)
16    E701 multiple statements on one line (colon)
10    E713 test for membership should be 'not in'
3     E714 test for object identity should be 'is not'
21    F401 'twisted.internet.protocol.Factory' imported but unused
2     F811 redefinition of unused 'error' from line 11
2     F821 undefined name 'pn'
8     F841 local variable 'start' is assigned to but never used
42    W291 trailing whitespace
607   W293 blank line contains whitespace
15    W391 blank line at end of file

[oberstet]

of the warning above, this might actually be bugs:

(cpy361_7) oberstet@thinkpad-t430s:~/scm/3rdparty/txdbus$ flake8 --select=F821 --show-source .
./txdbus/router.py:47:60: F821 undefined name 'pn'
                if m.path is None or not m.path.startswith(pn):
                                                           ^
./txdbus/test/test_marshal.py:39:20: F821 undefined name 'long'
            self.t(long(1),'x')
                   ^

[WhyNotHugo]

Yeah, I'd love flake8-compliance, but:

• Python3-compat comes first.
• Better coverage comes second.
• Not the project owner, so it's really @cocagne's call. (I'm all for it, FWIW).

I want something that passes 100% tests on all python versions before making non-critical changes, so that making them feels safer.

[WhyNotHugo]

Oh, the ones that are bug need addressing, of course. :)

[cocagne]

Agreed. It'd be a lot harder to understand where the project stands without a reliable unit test system to vet changes as they're made.

On Mon, Jun 5, 2017 at 10:07 AM, Hugo Osvaldo Barrera < notifications@github.com> wrote:

Yeah, I'd love flake8-compliance, but:

• Python3-compat comes first.
• Better coverage comes second.
• Not the project owner, so it's really @cocagne https://github.com/cocagne's call. (I'm all for it, FWIW).

I want something that passes 100% tests on all python versions before making non-critical changes, so that making them feels safer.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/cocagne/txdbus/issues/11#issuecomment-306212358, or mute the thread https://github.com/notifications/unsubscribe-auth/ABrLfLdm5CAFHKF6jWdY9rFqxRUlW1iRks5sBBnNgaJpZM4B5xdc .

[nanonyme]

To be fair, swapping first two around might not be a bad idea either. You can't really know if the Python3 port is complete unless you have very high coverage

[WhyNotHugo]

I want something that passes 100% tests with great coverage before making non-critical changes, so that making them feels safer.

They kinda go hand in hand. I didn't mean to give an order (although I literally did, heh!). But I'd rather see currently skipped tests pass on python3 before adding new ones.

[cocagne]

Coverage is pretty good already. Its somewhere in the ballpark of 95% or so. I had originally intended to hit 100% but the burden of injecting failures in some parts of the code just wasn't worth the effort. The initial test implementation tried to omit coverage only on low-risk, high-burden areas but there are almost certainly areas missed over the years that could use some shoring up. Still, it's in pretty good shape at present.

On Mon, Jun 5, 2017 at 10:11 AM, Seppo Yli-Olli notifications@github.com wrote:

To be fair, swapping first two around might not be a bad idea either. You can't really know if the Python3 port is complete unless you have very high coverage

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/cocagne/txdbus/issues/11#issuecomment-306213314, or mute the thread https://github.com/notifications/unsubscribe-auth/ABrLfL5tTVlCUH5iYAzoF258NKderRY0ks5sBBqVgaJpZM4B5xdc .

[WhyNotHugo]

At what point can we close this? All tests are currently passing (including client ones with the native DBus and the builtin one). Some examples might need an update, but other than that, this seems almost done (more coverage is always welcome, though).

[oberstet]

Looking at the example on the frontpage, I'm a bit puzzled that none of the arguments there is using explicit b'..' or u'...'. So the arguments will be bytes on Py2 and unicode on Py3. Are the arguments auto-coerced under the hood?

Take

    notifier = yield con.getRemoteObject('org.freedesktop.Notifications',
                                         '/org/freedesktop/Notifications')

Does getRemoteObject take bytes or strings?

[oberstet]

@hobarrera I only have sketchy knowledge about dbus and stuff, haven't looked deeply enough into the txdbus codebase, but are the unit tests actually talk to a real dbus? and if so, are there tests that

• roundtrip Py2 -> dbus -> Py2 (and Py3 -> dbus -> Py3) - eg both a proc implemented in Py and called from Py
• roundtrip Py2 -> dbus -> Py3 (and Py3 -> dbus -> Py2)

Probably these are more integration, rather than unit tests, but hope you get what I mean here ..

[WhyNotHugo]

@oberstet Like I said before, there are tests that both the native DBus, and txdbus's one as well. I believe there's one that listens on a signal and tests that firing it works as expected, for example? Are those the sort you're talking about?

See here for more details.

[cocagne]

@hobarrera Now that all of the unit tests are passing, I'm okay with declaring this as "done" when one of you guys tells me you feel confident that it's "done". Having all the unit tests pass has been my gating criteria for getting changes into production so far; though, admittedly, I've been somewhat less than rigorous about resolving the intermittent failures (thanks @hobarrera and @exvito for shoring those up). A minor check outside the unit tests like running the readme's example application on Py3 and seeing the expected result would be good but I'll leave the decision up to you guys. I have yet to make the switch to Py3 so you'll be in a better position to judge its readiness than I.

On Mon, Jun 5, 2017 at 2:00 PM, Hugo Osvaldo Barrera < notifications@github.com> wrote:

@oberstet https://github.com/oberstet Like I said before, there are tests that both the native DBus, and txdbus's one as well. I believe there's one that listens on a signal and tests that firing it works as expected, for example? Are those the sort you're talking about?

See here https://github.com/cocagne/txdbus/blob/master/tests/client_tests.py for more details.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/cocagne/txdbus/issues/11#issuecomment-306274134, or mute the thread https://github.com/notifications/unsubscribe-auth/ABrLfMTTv4A3hcDoDTuOD-ao9UdYctD_ks5sBFA4gaJpZM4B5xdc .

[exvito]

@cocagne,

Thanks for the comment. I'm very conservative with changes and code/systems in general, thus I share some of the concerns @oberstet raised.

Being only slightly familiar with the codebase I can confirm at least that the current tests cover the full native round-trip Python > DBus > Python, but only among the same Python versions, not across different ones. These are implemented in client_tests.py and driven by test_client_against_native_bus.py. Interestingly, an alternative driver for such tests, bypassing communication with a system provided DBus is implemented in test_client_against_internal_bus.py and is currently disabled (I tried enabling it, but found no quick solution for succesfully running this complementary set of tests).

What drove me to improve txdbus is a very particular use case of using RFCOMM bluetooth sockets on a Raspberry PI: this is done over DBus, talking to the services provided by BlueZ, and working fine for me (@oberstet, this is the one I won in last year's EuroPython giveaway by crossbar.io -- thanks, it's an amazing little system).

Having said this, I suggest that:

• The test @oberstet identified (test_long) should be reviewed before closing this -- the current implementation is currently running and passing on Python 3 which does not make much sense, given that it's doing nothing. It should be skipped instead.
• Line https://github.com/cocagne/txdbus/blob/master/txdbus/router.py#L47 really seems to contain a reference to an unknown variable pn... I have no idea what that is, but feel this needs fixing -- not related to Python 3. Maybe a new issue should be created instead.
• With regards to improving the codebase via linting and increased coverage, again, I feel should be handled progressively under independent issues / PRs.

This approach ensures each issue / PR is as focused as possible and brings in the smallest amount of changes at a time -- I guess I can all agree that is a good policy. :)

In short:

• Create a new issue skip the test_long in Python 3 and call this ticket complete once that lands.
• Create new issue to fix the "unknown variable" @oberstet identified in router.py -- unrelated to Python 3 -- @cocagne, maybe you can help out a bit here if this is still familiar to you?
• Create many issues -- maybe one per module? -- to improve linting results.
• Create one issue to prepare for a new PyPI release -- updating setup.py and associated metadata.
• Release, letting people out there use txdbus on Python 3. Now that there's better infrastructure in place, any regressions should be easier to identify and address. :)

[oberstet]

@exvito ah, nice: hello again;) fwiw, I agree with basically everything you say above.

@hobarrera @cocagne Right now, I don't feel confident that Python 3 is properly supported.

@hobarrera What I meant with round-tripping is between different Python versions. Like have a procedure implemented in Python 3, exposed via Dbus, and being called from Python 2. Where the procedure name is a fancy Unicode name, and/or the procedure takes an Unicode argument (and the test actually uses non-US-ASCII characters) and/or the procedure takes a binary argument (and the test actually uses a binary value which isn't valid UTF8). I've spent many many hours on UTF8 (in the context of WebSocket) and Python 2/3 issues .. and these are the kind of tests that will really get to the bottom. For example, Autobahn uses its own UTF8 validator because the Python built in one does NOT catch all invalid UTF8! So I think I know what I am talking about ..

---

Tbh, I don't even understand why the example on the landing page works because it doesn't use literal markers.

I had a short look into the code base, and the type checking is problematic.

Take this line:

    if not isinstance(var, six.string_types):
raise MarshallingError('Required string. Received: ' + repr(var))

The problem is, on Python 2 this will always evaluate to False! For both bytes and unicode strings.

(cpy361_7) oberstet@thinkpad-t430s:~/scm/crossbario/crossbar-fabric-device-agent$ python2
Python 2.7.12 (default, Nov 19 2016, 06:48:10) 
[GCC 5.4.0 20160609] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import six
>>> six.text_type
<type 'unicode'>
>>> six.binary_type
<type 'str'>
>>> type(b'hello') == six.binary_type
True
>>> type(b'hello') == six.text_type
False
>>> six.string_types
(<type 'basestring'>,)
>>> isinstance(u'hello', six.string_types)
True
>>> isinstance(b'hello', six.string_types)
True
>>> 
(cpy361_7) oberstet@thinkpad-t430s:~/scm/crossbario/crossbar-fabric-device-agent$ python
Python 3.6.1 (default, Apr  2 2017, 18:19:20) 
[GCC 5.4.0 20160609] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import six
>>> six.text_type
<class 'str'>
>>> six.binary_type
<class 'bytes'>
>>> type(b'hello') == six.binary_type
True
>>> type(b'hello') == six.text_type
False
>>> six.string_types
(<class 'str'>,)
>>> isinstance(u'hello', six.string_types)
True
>>> isinstance(b'hello', six.string_types)
False
>>> 

The reason is, that on Python 2, both str (bytes) and unicode derive of basestring (https://pythonhosted.org/six/#six.string_types, https://docs.python.org/2/library/functions.html#basestring).

The right way to check is via six.text_type and six.binary_type. See above.

[WhyNotHugo]

Would you guys mind creating individual issues for the things mentioned about, to individually track/address them?

Also, if you can test your software using it on python3, and see any obvious/simple bugs, that'd be great (assuming your codebases are python3-compatible already).

[exvito]

@hobarrera: I just created the issues we recently identified.

While I tend to side with @oberstet in his appreciation of Python 3 support, I suggest we go address #61, close this issue [1], then release in #64 -- holding off any other changes until then.

[1] We probably should create a new "Review/Improve Python 3 support" issue linking to @oberstet comments which are very relevant.

Again, my underlying idea is to "share soon" to get feedback "soon" -- not claiming Python 3 support is 100% perfect.

[oberstet]

@hobarrera I've created separate issues for the stuff that concerns me - so from my side, we could close this one.

[WhyNotHugo]

Again, my underlying idea is to "share soon" to get feedback "soon" -- not claiming Python 3 support is 100% perfect.

Yeah, "experimental support" sounds okay.

[exvito]

@hobarrera,

Do you agree with the proposed way forward I commented above? If you do, please state that, otherwise please share your thoughts.

My intent is simple:

• Have an updated release on PyPI soon enough.
• Minimize regressions in future fixes and improvements.

:)

[exvito]

Progress:

• 61 is done, via #65.

• Several issues were spun-off from the recent discussion - good.

Next:

• Close this issue?
• Shall we focus on #64 (release) before anything else?

[WhyNotHugo]

Closing this. We've all agreed that experimental support is in-place, and have other issues to track other stuff.

If there's something missing, feel free to open a new issue for wherever it is. :)