[Freeipa-devel] [freeipa PR#394][comment] Add fix for ipa plugins command
URL: https://github.com/freeipa/freeipa/pull/394 Title: #394: Add fix for ipa plugins command MartinBasti commented: """ Fixed upstream master: https://fedorahosted.org/freeipa/changeset/b3c41f21e51e5389d95b5486dcdfdc3f9a8b0424 """ See the full comment at https://github.com/freeipa/freeipa/pull/394#issuecomment-280599222 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#394][comment] Add fix for ipa plugins command
URL: https://github.com/freeipa/freeipa/pull/394 Title: #394: Add fix for ipa plugins command MartinBasti commented: """ That test actually doesn't test output of command, IMO it should be xmlrpc_test. But it can be done later, shouldn't block this PR """ See the full comment at https://github.com/freeipa/freeipa/pull/394#issuecomment-279794064 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#394][comment] Add fix for ipa plugins command
URL: https://github.com/freeipa/freeipa/pull/394 Title: #394: Add fix for ipa plugins command MartinBasti commented: """ That test actually doesn't test output of command, IMO it should be xmlrpc_test. But it can be done later, shouldn't block this PR """ See the full comment at https://github.com/freeipa/freeipa/pull/394#issuecomment-279794064 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#394][comment] Add fix for ipa plugins command
URL: https://github.com/freeipa/freeipa/pull/394 Title: #394: Add fix for ipa plugins command MartinBasti commented: """ @Akasurde sorry for delay, we still miss test. Otherwise I'm fine with this approach (when issue commented inline fixed) """ See the full comment at https://github.com/freeipa/freeipa/pull/394#issuecomment-277977077 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#394][comment] Add fix for ipa plugins command
URL: https://github.com/freeipa/freeipa/pull/394 Title: #394: Add fix for ipa plugins command Akasurde commented: """ @MartinBasti is there any action item pending on my side? """ See the full comment at https://github.com/freeipa/freeipa/pull/394#issuecomment-277606801 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#394][comment] Add fix for ipa plugins command
URL: https://github.com/freeipa/freeipa/pull/394 Title: #394: Add fix for ipa plugins command MartinBasti commented: """ I would like to have `py3 str` <=> `py2 unicode`, `py3 bytes` <=> `py2 str`, but framework is far away from this ideal state. So I have no strong opinion, and once we will drop py2, so I'm not sure if we want to migrate everything in py2 to unicode if it work in other cases. """ See the full comment at https://github.com/freeipa/freeipa/pull/394#issuecomment-273450333 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#394][comment] Add fix for ipa plugins command
URL: https://github.com/freeipa/freeipa/pull/394 Title: #394: Add fix for ipa plugins command HonzaCholasta commented: """ We are OK with the patch because fixing the root cause is out of the scope of this PR. """ See the full comment at https://github.com/freeipa/freeipa/pull/394#issuecomment-273448687 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#394][comment] Add fix for ipa plugins command
URL: https://github.com/freeipa/freeipa/pull/394 Title: #394: Add fix for ipa plugins command HonzaCholasta commented: """ @tiran, namespace keys are always ASCII. But feel free to open a ticket to convert all remaining uses of `str` as text to `unicode`, changing it for one random bit in this unrelated PR isn't particularly helpful when you take the big picture into account. """ See the full comment at https://github.com/freeipa/freeipa/pull/394#issuecomment-273429342 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#394][comment] Add fix for ipa plugins command
URL: https://github.com/freeipa/freeipa/pull/394 Title: #394: Add fix for ipa plugins command tiran commented: """ In Python 2 str is a Chimera with the head of a text object and the body of a bytes object. It's just text if all text you got is ASCII. For clean polyglot code it's highly recommended to avoid Python 2 str and use Python 2's unicode for all text. Most of FreeIPA's Python code has been adopted to unicode for text very well. This one of the few places that slipped through. The benefits are consistent treatment of text as Python 2 unicode, which leads to a proper fix instead of a patch (in this case decoding with six.text_type). """ See the full comment at https://github.com/freeipa/freeipa/pull/394#issuecomment-273424928 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#394][comment] Add fix for ipa plugins command
URL: https://github.com/freeipa/freeipa/pull/394 Title: #394: Add fix for ipa plugins command HonzaCholasta commented: """ The namespace keys *are* text (`str`) in both Python 2 and 3. The issue here is that the RPC layer assumes that `str` is binary data, which the patch correctly fixes by converting the keys to `unicode` before they enter the RPC layer. There is no benefit in making the keys themselves `unicode`. """ See the full comment at https://github.com/freeipa/freeipa/pull/394#issuecomment-273422664 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#394][comment] Add fix for ipa plugins command
URL: https://github.com/freeipa/freeipa/pull/394 Title: #394: Add fix for ipa plugins command tiran commented: """ Why should *Python 2 class names are str instances* prevent us from making the namespace keys text? In Python 2 ASCII str and ASCII unicode are equivalent dict keys (same hash, compare equaly). In Python 3 the keys are going to be text anyway. """ See the full comment at https://github.com/freeipa/freeipa/pull/394#issuecomment-273420986 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#394][comment] Add fix for ipa plugins command
URL: https://github.com/freeipa/freeipa/pull/394 Title: #394: Add fix for ipa plugins command tiran commented: """ Why should *Python 2 class names are str instances* prevent us from making the namespace keys text? In Python 2 ASCII str and ASCII unicode are equivalent dict keys (same hash, compare equaly). In Python 3 the keys are going to be text anyway. """ See the full comment at https://github.com/freeipa/freeipa/pull/394#issuecomment-273420986 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#394][comment] Add fix for ipa plugins command
URL: https://github.com/freeipa/freeipa/pull/394 Title: #394: Add fix for ipa plugins command Akasurde commented: """ @martbab Yes, I will write a test case for this scenario and attach here. """ See the full comment at https://github.com/freeipa/freeipa/pull/394#issuecomment-273420038 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#394][comment] Add fix for ipa plugins command
URL: https://github.com/freeipa/freeipa/pull/394 Title: #394: Add fix for ipa plugins command martbab commented: """ @Akasurde are you OK with writing a simple regression test for this command as a part of this PR? """ See the full comment at https://github.com/freeipa/freeipa/pull/394#issuecomment-273416076 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#394][comment] Add fix for ipa plugins command
URL: https://github.com/freeipa/freeipa/pull/394 Title: #394: Add fix for ipa plugins command HonzaCholasta commented: """ @mbasti-rh, no. Classes aren't named using unicode strings either. """ See the full comment at https://github.com/freeipa/freeipa/pull/394#issuecomment-273395287 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#394][comment] Add fix for ipa plugins command
URL: https://github.com/freeipa/freeipa/pull/394 Title: #394: Add fix for ipa plugins command mbasti-rh commented: """ Shouldn't be namespaces named using unicode strings? """ See the full comment at https://github.com/freeipa/freeipa/pull/394#issuecomment-273275568 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#394][comment] Add fix for ipa plugins command
URL: https://github.com/freeipa/freeipa/pull/394 Title: #394: Add fix for ipa plugins command rcritten commented: """ How about a test to prevent future regressions? """ See the full comment at https://github.com/freeipa/freeipa/pull/394#issuecomment-272962038 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [freeipa PR#394][comment] Add fix for ipa plugins command
URL: https://github.com/freeipa/freeipa/pull/394
Title: #394: Add fix for ipa plugins command
martbab commented:
"""
Thanks the patch makes the command work. However, the namespace names are
returned as string, not unicode literals and thus the framework returns them as
base64-encoded values:
```
# ipa plugins
ipaclient.plugins.automember.automember_add_condition: Q29tbWFuZA==, TWV0aG9k
ipaclient.plugins.automount.automountlocation_import: Q29tbWFuZA==
ipaclient.plugins.automount.automountlocation_tofiles: Q29tbWFuZA==, TWV0aG9k
# echo 'Q29tbWFuZA==' | base64 -d && echo
Command
# echo '# echo 'TWV0aG9k' | base64 -d && echo
Method
```
One way to fix this is to wrap namespace name in `six.test_type`, this should
work in both py2 and py3:
```diff
diff --git a/ipalib/misc.py b/ipalib/misc.py
index 264ec29..6234961 100644
--- a/ipalib/misc.py
+++ b/ipalib/misc.py
@@ -3,6 +3,9 @@
#
import re
+
+import six
+
from ipalib import LocalOrRemote, _, ngettext
from ipalib.output import Output, summary
from ipalib import Flag
@@ -124,7 +127,7 @@ class plugins(LocalOrRemote):
for plugin in self.api[namespace]():
cls = type(plugin)
key = '{}.{}'.format(cls.__module__, cls.__name__)
-result.setdefault(key, []).append(namespace)
+result.setdefault(key, []).append(six.text_type(namespace))
return dict(
result=result
```
"""
See the full comment at
https://github.com/freeipa/freeipa/pull/394#issuecomment-272905328
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
