pysipp
pysipp copied to clipboard
Drop py2.7 and add trio support!
This is in preparation of a python 3.6+ only 1.0.0.alpha release and replaces all the internal processing launching machinery with the new subprocess support in trio.
This not only gets us cross OS support for free but also gives access to async/await based scenario and agent spawning.
A few interesting notes:
SIPpturns out to be a good fit withtriocancellation semantics since it has a built in cancellation system viaSIGUSR1- this now gives us a strictly single threaded implementation
triogives us deterministic teardown allowing for much finer control over per agent failures and subsequent reporting
TODO:
- write some async spawning tests and examples in the readme
- consider playing around with timeout and other cancellation tools such as
trio.move_on_after()andtrio.fail_after()and figure out how this can interplay with log reporting for interactive use - add the mac os env to CI
I look forward to feedback and thoughts!
ping @y-luis
Turns out we need to wait for the next version trio to be released before the subprocess stuff lands.
Back to only one test failing; the one for async launching which needs a rewrite and new docs for how to do with this trio.
hello, is there anything we can do to help merge this PR?
@kontsaki my bad, I've let this slip quite a while 😿
I'm not currently working in telephony any more so haven't had any dire needs for this enhancement.
is there anything we can do to help merge this PR?
Testing and user feedback on the ergonomics would be good.
We were supposed to do a proper release a while back. I can't remember what the hold up was (it could have just been me?).
It would be good to get some further user feedback here on this though.
@goodboy want me to open a PR for this branch if i get the chance and fix what you have commented?
want me to open a PR for this branch
@kontsaki yeah 💯 that'd be amazing
We can use this branch to make some effort towards modernizing the codebase for #62 as well 🏄🏼
Actually just let me rebase this onto master first before you get started.
@kontsaki there you go.
@goodboy is it okay if i fix some pylint warnings and/or run the black formatter ?
is it okay if i fix some pylint warnings and/or run the black formatter
@kontsaki do it in another PR if you want. Try to keep this one focussed around the new process launching and trio.
@kontsaki I might just get this onto master as a new dev release so that interested peeps can try it out.
Just need to tweak the release number.. Also would be good to get a changelog going proper.
Welp merged #67 and now the rebase on this branch will be a nightmare 😂
Not sure I'll get to it today.
@kontsaki unfortunately that means #65 can't really be simplified yet either until this branch is fixed.
This is why i normally don't do big formatting changes to someone else's code base when trying to get new features in 😉
We'll get through it eventually.
i will fix the conflicts
@kontsaki try doing a git rebase master from your copy of this branch you'll see what I mean.
There's quite a few tweaks that are needed and afaik it can't be done wholesale using a --ours flag.
@kontsaki ok ur black changes should be in here now so let's take a look at #65 🥳
Yah so this merge commit is not only imo messy it makes the history more confusing for onlookers.
@kontsaki take a read of this https://stackoverflow.com/a/13194092
The main reason i would require this is not just for (as some people think is the only benefit),
The only benefit of rebase strategy is linear history and that's it.
If mainline is desynced by a few commits it's nbd, but it's when you're making changes to every file in the repo (i.e. in the case of #69) it's much easier to manage the history and see the origin of broad changes if those changes are isolated in a linear path of the git tree.

Now there are paths both to master and away from master - this imo is confusing especially if it gets repeated recursively.
For example now if you don't rebase #65, and instead merge, we'll have yet another path going back out again which creates history cycles and then becomes harder to follow by manually inspecting commit history.
i use a git alias: alias.logtree=log --graph --oneline --decorate --all which if you look at the repo history you'll see what i mean about avoiding away from mainline cycles in the history.
This (in imo) the cleanliness of rebasing and it comes at really no extra cost except forcing you to go over changes on your branch and potentially clean up your commits.
Can be done nicely with git rebase -i in your editor as well 😉
Before we merge this I'd like to get some feedback from any users that are willing to alpha test it as well.
I don't think there's much risk bringing it into master, but also don't want to break anyone's setups who are pulling from git (since we used to not have a pypi package not that long ago). There's going to be some trio related version requirements for example I'd imagine.
If we get silence or feedback that everything is fine, we'll just merge and figure out peeps problems incrementally.
@kontsaki are you happy with this patch set?
I see you closed #65, which actually I think is better for right now until we can get better idea of what runner api everyone prefers. I mentioned this a bit in the comments on that PR:
I'm curious to know if anyone has actually used this branch for real testing; that would make it much easier to merge and move on.
@goodboy are you willing to rebase this PR?