[strongSwan-dev] Python vici library

[Björn Schuberg]

Hi Martin,

Nice start on the README. I took a moment to make the python part a bit
more "pythonic", and pushed it to a branch on github:
https://github.com/sbjorn/strongswan/commit/f6672dcfdc7eaf39ae8952c9c328068e1d7ca3ec
https://github.com/sbjorn/strongswan/commit/7b78cef93dd9c0b37401b9d359c634050a8e9ef0

I've taken a look at your code changes. I definitely agree that implicit
error checking is nicer, and it will make the experience of using the
library much smoother. There is a problem with your implementation of [2]
however, you are discarding any errors for commands that require event
streaming -- only the events are returned -- there is no way to know
whether the command was successful.

While generators is a good idea for response streaming, it is not a great
fit here. The current implementation works on the assumption that the
generator is exhausted before another command starts. There is no way to
guarantee this, as the user may draw elements from the generator as he/she
wishes. This will make the consecutive command fail in best case, and in
the worst case, return the wrong result. Another problem is that no errors
are reported due to [2].

Here is an example:

>>> import vici
>>> s = vici.Session()

>>> g = s.list_certs()
>>> msg = next(g)
>>> print s.version()

Traceback (most recent call last):
  File "scr.py", line 6, in <module>
    print(s.version())
  File "vici/session.py", line 21, in version
    return self.handler.request("version")
  File "vici/session.py", line 238, in request
    response=Packet.CMD_RESPONSE
vici.exception.SessionException: Unexpected response type 7, expected '1'
(CMD_RESPONSE)


For this to be implemented properly, the way the library currently send and
receive messages must be rethought.


Kind regards,
Björn

2015-03-02 15:45 GMT+01:00 Martin Willi <martin at strongswan.org>:

> Björn,
>
> > Your README.rst is not yet part of it, probably we should integrate
> > that to the vici README.md.
>
> I've updated our vici README.md [1] to include some basic steps to get
> started with the python library.
>
> In addition, I'd like to propose a minor change to the main Session
> command calls. Instead of returning a CommandResult, I'd prefer to raise
> a CommandException if a command failed [2].
>
> While I think implicit error checking with exceptions makes sense, the
> main reason for that change is that it allows us to return generators
> for streamed objects [3]. I think that is favorable, especially as some
> longer lasting commands return immediate feedback when called
> interactively. For example, the initiate() command directly returns
> streamed log messages, long before the command finally succeeds or
> fails.
>
> Please let me know what you think about these changes.
>
> Best regards
> Martin
>
> [1]
> https://github.com/strongswan/strongswan/blob/vici-python/src/libcharon/plugins/vici/README.md#vici-python-egg
> [2]http://git.strongswan.org/?p=strongswan.git;a=commitdiff;h=713d2f42
> [3]http://git.strongswan.org/?p=strongswan.git;a=commitdiff;h=ec599c65
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.strongswan.org/pipermail/dev/attachments/20150306/d1b1674a/attachment.html>