Description of problem: Version-Release number of selected component (if applicable): How reproducible: Steps to Reproduce: 1. 2. 3. Actual results: Expected results: Additional info:
Please revert this commit http://pkgs.fedoraproject.org/cgit/dbus-python.git/commit/?id=b00d726b5b3e8f7f590ed3eb95160d1b68003339 as it breaks the API by removing _dbus_class_table and replacing it with _dbus_interface_table
Patch's origin seems to be: https://bugs.freedesktop.org/show_bug.cgi?id=26903#c9 and according to git log, commit b00d726b5b3e8f7f590ed3eb95160d1b68003339 Author: Dan Mashal <dan.mashal> Date: Fri Jun 20 11:23:22 2014 -0700 Add patch to support object manager for Fedora server dbus api (requested by sgallagh) Will need to contact server workgroup to collaborate on how best to fix this.
sgallagh, Can you comment how the object_manager.patch to dbus-python is currently used (if at all)? If not, we can probably just drop it, otherwise, will need to come up with some better solution.
Thank you. Some additional informations: This API brakage prevent us to bring back bluemen in fedora. https://github.com/blueman-project/blueman/issues/161
Unfortunately, the rolekit project (a major feature of the Fedora Server effort) currently has a strict requirement on this patch. It's a major simplification to the API for creating objects at will (such as when we deploy roles and want to be able to manage them). For the first several months, we attempted to maintain a version of rolekit that would work with or without this patch in place, but it became too much burden (and since we didn't get any negative reports about the patch), we dropped the compatibility code and have subsequently built a lot of code that would be broken by reverting this. Also, I believe that firewalld is also taking advantage of this patch now (though that may be irrelevant once the firewalld conversion to C is complete).
CCing the writer of the original patch to see if he can figure out a way to maintain compatibility here.
I've also sent an email to upstream asking for their interpretation of whether _dbus_class_table should be considered public API or not.
I do not consider anything underscore-prefixed to be API. I'm also very tempted to say that if you have added distro patches to dbus-python that add new public API, you are not really running dbus-python any more, and you get to keep both pieces if it turns out not to be compatible with the upstream one... but the amount of time the relevant patch has been sitting in Bugzilla is not ideal either. Perhaps I should be applying the "you break it, you bought it" principle and declaring that Fedora's dbus-python maintainer is now the upstream :-)
I have considered _dbus_class_table to be internal to the implementation, not part of the API. This is what PEP-8, Python style guide, says. That is the reason I so carelessly changed it in the patch. The change there is to remove the _dbus_class_table dictionary keyed by class name to get interface table. The interface table for a class is now directly available as an attribute of the class. I find this much more straightforward. I am not familiar with bluemen. I looked at it to see it dynamically modifies the interface table add adds and removes there interfaces, methods and signals. I have not studied why. I wonder if there would be a better way to accomplish what ever it is doing. If it really must get to the interface table, and it has to support both old and new internal APIs, it could perhaps use something like (this is completely untested) interface_table = getattr(cls, '_dbus_interface_table') if interface_table is None: # revert to older internal API, if available class_table = getattr(cls, '_dbus_class_table') if class_table: interface_table = class_table.get(cls.__module__ + '.' + cls.__name__)) There is a slightly updated version of the patch now available in the upstream bug. It addresses some of the issues Simon found when reviewing the patch.
> I have considered _dbus_class_table to be internal to the implementation, > not part of the API. This is what PEP-8, Python style guide, says. I wouldn't say so. PEP 8 says the single underscore is for weak internal use. If a library uses this for objects of a module, I'd say they are not meant to be used by the user of the library ("internal to the module"). But if it is used for a class attribute, I'd say this attribute is not meant to be used on an object of that class ("internal to the class"), while using it within a derived class is internal use. I'd expect a double underscore if an attribute is also not meant to be used in a derived class. And at the very least I would not expect to see any non-API stuff in the API documentation [1]. [1] http://dbus.freedesktop.org/doc/dbus-python/api/dbus.service.Interface-class.html Anyway, API changes are valid. The patch can of course get applied upstream so that it is an official change. The only thing that bugs me a little is that Fedora's dbus-python "1.2.0" does not provide (a superset of) the official API of 1.2.0. > I am not familiar with bluemen. I looked at it to see it dynamically modifies the interface table add adds and removes there interfaces, methods and signals. I have not studied why. I wonder if there would be a better way to accomplish what ever it is doing. What it does is basically just adding methods and signals on interface "org.blueman.Applet" and path "/" (those parameters are actually always the same although the method signatures suggest something different). As this is a pretty usual use case and I don't see anything special in blueman's case there is definitely a more straightforward way to achieve this without the need of touching the table. I'll look into that as soon as I find the time.
This change also breaks apps using python-dbusmock for their unit tests. As python-dbusmock's classes are derived from the python-dbus classes, their use of the "private" API is legal, IMHO.
Concerning blueman we avoid this issue now by re-running dbus.service.InterfaceType.__init__ instead of modifying the table ourselves when adding or removing methods. https://github.com/blueman-project/blueman/pull/170
This package has changed ownership in the Fedora Package Database. Reassigning to the new owner of this component.
(In reply to Stephen Gallagher from comment #7) > I've also sent an email to upstream asking for their interpretation of > whether _dbus_class_table should be considered public API or not. I intend to revert the patch, if you want the new object class you will need to fork dbus-python and package it yourself!
(In reply to Stephen Gallagher from comment #7) > I've also sent an email to upstream asking for their interpretation of > whether _dbus_class_table should be considered public API or not. Reverted for F24 only, this should give you plenty of time to fork and package it.
Sad to see it go. The patch is not upstream and undoubtedly the change to _dbus_class_table is a good reason not to include it there. As I noted in the upstream comment https://bugs.freedesktop.org/show_bug.cgi?id=26903#c15 the patch bundles several changes. The replacing of _dbus_class_table with _dbus_interface_table is just a convenience and not something that is mandatory to do to implement the property decorator. Unfortunately I have no use for the patch and I can not justify myself to spend time on it. I hope if anyone has use for the decorator, they rewrite the patch accordingly and attach it to the upstream bug.
(In reply to leigh scott from comment #15) > > (In reply to Stephen Gallagher from comment #7) > > I've also sent an email to upstream asking for their interpretation of > > whether _dbus_class_table should be considered public API or not. > > Reverted for F24 only, this should give you plenty of time to fork and > package it. Leigh, the rolekit developers are in the middle of converting away from using python-dbus and over to gdbus instead. Once this is done, there will be no need for the fork. Would it be possible for you to keep those patches present for just a while longer while we make this transition? It would be rather painful for us to have to maintain a fork for just a few months until we can drop it. We will keep our progress updated on https://github.com/libre-server/rolekit/issues/24 for you to follow. It's a fairly high priority for us and we do want it to happen for Fedora 24. In the meantime, though, it would be preferable if things were not broken on Rawhide until we make the switch. Would that be a reasonable compromise for you?
(In reply to Stephen Gallagher from comment #17) > Would that be a reasonable compromise for you? Yes
Good news, Leigh! Thomas Woerner found a workaround[1] for the problem by extending the Introspect() method instead of modifying python-dbus itself. It's not a true fix, but it will work for our purposes and avoid a costly migration to a new D-BUS implementation. I'm going to do a new build of rolekit sometime early next week which will include this fix for both F23 and F24. I'll let you know when it happens, but as a result we won't need to carry (and maintain) this patch for the life of Fedora 23. Of course, rolekit and firewalld may not be the only ones relying on this patch right now, so I'm hoping that anyone with an opinion on this will chime in on this thread before Fedora 23 Final Freeze. Leigh, thank you for bearing with us while we navigated this issue. It's appreciated.
https://bodhi.fedoraproject.org/updates/rolekit-0.4.0-5.fc23 has now been submitted to Bodhi which no longer requires the patched python-dbus. I'm not sure if firewalld is ready with its version of the patch, so I'm putting NEEDSINFO on Thomas Woerner to chime in. But I think that should be coming along quite soon as well.
firewalld is not depending on the properties patch in dbus-python. I am working on an enhanced version of the property handling of rolekit for firewalld. In firewalld some properties are writeable, therefore the property handling needs to be more powerful than in rolekit. The plan is to integrate the final property handling into slip.dbus.service. Then it will be available for all consumers of python-slip.
This message is a reminder that Fedora 21 is nearing its end of life. Approximately 4 (four) weeks from now Fedora will stop maintaining and issuing updates for Fedora 21. It is Fedora's policy to close all bug reports from releases that are no longer maintained. At that time this bug will be closed as EOL if it remains open with a Fedora 'version' of '21'. Package Maintainer: If you wish for this bug to remain open because you plan to fix it in a currently maintained version, simply change the 'version' to a later Fedora version. Thank you for reporting this issue and we are sorry that we were not able to fix it before Fedora 21 is end of life. If you would still like to see this bug fixed and are able to reproduce it against a later version of Fedora, you are encouraged change the 'version' to a later Fedora version prior this bug is closed as described in the policy above. Although we aim to fix as many bugs as possible during every release's lifetime, sometimes those efforts are overtaken by events. Often a more recent Fedora release includes newer upstream software that fixes bugs or makes them obsolete.
(In reply to Stephen Gallagher from comment #20) > https://bodhi.fedoraproject.org/updates/rolekit-0.4.0-5.fc23 has now been > submitted to Bodhi which no longer requires the patched python-dbus. > > I'm not sure if firewalld is ready with its version of the patch, so I'm > putting NEEDSINFO on Thomas Woerner to chime in. But I think that should be > coming along quite soon as well. So I'm ok to remove the patch for >= 23 releases?
(In reply to leigh scott from comment #23) > (In reply to Stephen Gallagher from comment #20) > > https://bodhi.fedoraproject.org/updates/rolekit-0.4.0-5.fc23 has now been > > submitted to Bodhi which no longer requires the patched python-dbus. > > > > I'm not sure if firewalld is ready with its version of the patch, so I'm > > putting NEEDSINFO on Thomas Woerner to chime in. But I think that should be > > coming along quite soon as well. > > So I'm ok to remove the patch for >= 23 releases? Yes, please go ahead and remove it. Thank you for your patience in this matter.
dbus-python-1.2.0-12.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2015-b5c42d1d47
dbus-python-1.2.0-12.fc23 has been pushed to the Fedora 23 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with $ su -c 'dnf --enablerepo=updates-testing update dbus-python' You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2015-b5c42d1d47
dbus-python-1.2.0-12.fc23 has been pushed to the Fedora 23 stable repository. If problems still persist, please make note of it in this bug report.