On 24.3.2016 14:13, Martin Babinsky wrote:
On 03/24/2016 01:47 PM, Martin Babinsky wrote:
On 03/22/2016 12:28 PM, Martin Babinsky wrote:
On 03/16/2016 02:17 PM, Martin Babinsky wrote:
On 03/16/2016 01:35 PM, Nathaniel McCallum wrote:
On Wed, 2016-03-16 at 07:25 +0100, Jan Cholasta wrote:
On 15.3.2016 22:22, Nathaniel McCallum wrote:
On Tue, 2016-03-15 at 17:54 +0100, Martin Babinsky wrote:
On 03/15/2016 03:36 PM, Martin Babinsky wrote:
On 03/09/2016 07:06 AM, Jan Cholasta wrote:
On 8.3.2016 17:45, Martin Babinsky wrote:
On 03/08/2016 05:35 PM, Jan Cholasta wrote:
Hi,
On 8.3.2016 16:21, Martin Babinsky wrote:
https://fedorahosted.org/freeipa/ticket/5700
1) Instead of checking for utf-8 in particular, I would
prefer a more
robust approach:
try:
qr = qrcode.QRCode()
qr.add_data('test')
qr.make()
qr.print_ascii(tty=True)
except UnicodeError:
# it is not printable
else:
# it is printable
Now you mean the check in the _check_qrcode_capability() or
the
_print_qrcode() method itself?
_check_qrcode_capability() of course.
2) There is no os.isatty() check to see if stdout is
actually
a tty.
This check is performed inside both print_ascii() and
print_tty()
methods of QRCode object, but you probably mean that I
should
put the
check also into _check_qrcode_capability() method, right?
Yes. If stdout is not a tty, we should at least not tty=True
in
print_ascii().
Honza
Attaching updated patch. After the discussion with other
developers
we
decided to just print warnings when non-UTF-8 encoding is used
and
tty
width is smaller that the QR code size.
Found some minor errors in the patch, attaching updated version.
NACK
This patch has the major problem that tokens are added but then
unusable because they can't be provisioned to the devices. You need
to
check if qrcode output is possible before the token is added to
LDAP.
We discussed this on the IPA devel meeting and the decision was that
since the otpauth URI is always displayed, a warning is sufficient
when
the QR code cannot be printed.
If you disagree, could you explain why the URI is not sufficient for
provisioning the token?
I guess that is okay.
Thank you Nathaniel.
Jan had some offline comments to the patch. Attaching updated version.
Attaching updated patches.
I fixed the warning message when the QR code can not be rendered.
Attaching updated patches.
6th time the charm.
Thanks, ACK.
Pushed to:
master: 7febe569cede47b50a0ee1b19968627716ddbc0d
ipa-4-3: 77e9d31c75f7514f076662ac4e3ffcf66915880f
--
Jan Cholasta
--
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
