<div dir="ltr">Hi Martin,<div><br></div><div>Thanks for your update. It looks a bit complicated to me, but I've tested your changes, and it is seems to work! Let's go with this, even though I'm a bit skeptical to the limitations incurred.<br></div><div><br></div><div>Also, I've just opened a pull request with python 3 support that I worked on yesterday.</div><div><a href="https://github.com/strongswan/strongswan/pull/10">https://github.com/strongswan/strongswan/pull/10</a><br></div><div><br></div><div><br></div><div>Cheers,</div><div>Björn</div></div><div class="gmail_extra"><br><div class="gmail_quote">2015-03-09 12:42 GMT+01:00 Martin Willi <span dir="ltr"><<a href="mailto:martin@strongswan.org" target="_blank">martin@strongswan.org</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Björn,<br>
<br>
Thanks for reviewing these changes.<br>
<span class=""><br>
> Nice start on the README. I took a moment to make the python part a bit<br>
> more "pythonic", and pushed it to a branch on github:<br>
<br>
</span>Thanks for the fixes, I've squashed them into the associated commits in<br>
my branch.<br>
<span class=""><br>
> I've taken a look at your code changes. I definitely agree that<br>
> implicit error checking is nicer, and it will make the experience of<br>
> using the library much smoother. There is a problem with your<br>
</span>> implementation however, you are discarding any errors for commands that<br>
> require event streaming<br>
<br>
Correct, I've tried to address this with [1].<br>
<span class=""><br>
> While generators is a good idea for response streaming, it is not a great<br>
> fit here. The current implementation works on the assumption that the<br>
> generator is exhausted before another command starts. There is no way to<br>
> guarantee this, as the user may draw elements from the generator as he/she<br>
> wishes.<br>
<br>
</span>While I agree at some point, IMHO this is how generators in Python work.<br>
The user should call call close() on it, either implicitly by breaking<br>
from the iteration loop, or explicitly when manually iterating using<br>
next().<br>
<br>
Ideally, if the user does so we would unsubscribe from the events, and<br>
prematurely return from the associated command. However, this is<br>
currently not supported in vici at the daemon side, but we might extend<br>
that in the future.<br>
Instead, I'm just stopping calling yield once a GeneratorExit exception<br>
gets raised. This makes sure the protocol stays in sync, but stops<br>
returning results to the caller. This is implemented with [2].<br>
<span class=""><br>
> >>> g = s.list_certs()<br>
> >>> msg = next(g)<br>
> >>> print s.version()<br>
<br>
</span>While this example still fails, the following should work as expected:<br>
<span class=""><br>
>>> g = s.list_certs()<br>
>>> msg = next(g)<br>
</span>>>> g.close()<br>
>>> print s.version()<br>
<br>
I think with these changes, the advantages we get with the generator<br>
approach at least compensate for the disadvantages, given that most<br>
users iterate using "for" loops.<br>
<br>
Best regards<br>
Martin<br>
<br>
[1]<a href="http://git.strongswan.org/?p=strongswan.git;a=commitdiff;h=be353005" target="_blank">http://git.strongswan.org/?p=strongswan.git;a=commitdiff;h=be353005</a><br>
[2]<a href="http://git.strongswan.org/?p=strongswan.git;a=commitdiff;h=165603b5" target="_blank">http://git.strongswan.org/?p=strongswan.git;a=commitdiff;h=165603b5</a><br>
<br>
</blockquote></div><br></div>