Drop py2.7 and add trio support! #55

goodboy · 2019-07-11T02:31:43Z

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:

SIPp turns out to be a good fit with trio cancellation semantics since it has a built in cancellation system via SIGUSR1
this now gives us a strictly single threaded implementation
trio gives 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() and trio.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

Relates to #20

The `defaults` kwarg to `pysipp.agent.Scenario` isn't used so just drop it and instead comb through provided `kwargs` and pop out values passed by the user into `pysipp.agent.Scenario.defaults` making these the "new" defaults applied to agents. Also, rename the default settings dicts more explicitly. Resolves #20

These fancy tuple-attribute-properties are supposed to be strictly assigned tuple types; let's enforce that. Resolves #12

As per the change for #20, handle the case where the user passes in a `defaults` kwarg to `pysipp.scenario` and use it override the normal template. To fully solve #12 also support assignment of `None` which results in the tuple attribute taking a value of `(None, None)` implicitly.

After attempting to find an OS portable way to spawn subprocesses using the stdlib and coming out unsatisfied, I've decided use the new subprocess launching support in `trio`! This will of course require that the project moves to python 3.6+ giving us access to a lot of neat features of modern python including async/await support and adherence to the structured concurrency principles prominent in `trio`. It turns out this is a good fit since SIPp already has a built in cancellation mechanism via the SIGUSR1 signal. There's a lot of "core" changes to go over in this commit: - drop the "run protocol" and "runner creation" related hooks since they really shouldn't be overridden until there's some need for it and it's likely smarter to keep those "machinery" details strictly internal for now - the run "protocol" has now been relegated to an async function: `pysipp.launch.run_all_agents()` - many routines have been converted to async functions particularly at the runner (`pysipp.TrioRunner.run()`, `.get()`) and scenario (`pysipp.Scenario.arun()`) levels allowing us to expose both a sync and async interface for running subprocesses / agents - drop all the epoll/select loop stuff as this is entirely delegated to `trio.open_process()` and it's underlying machinery and APIs Resolves #53

goodboy · 2019-07-11T18:25:48Z

Turns out we need to wait for the next version trio to be released before the subprocess stuff lands.

goodboy · 2019-07-12T21:45:30Z

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.

kontsaki · 2020-12-24T10:53:27Z

hello, is there anything we can do to help merge this PR?

goodboy · 2020-12-24T13:33:09Z

@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 · 2020-12-24T13:35:29Z

pysipp/launch.py

-    Adheres to an interface similar to `multiprocessing.pool.AsyncResult`.
+class TrioRunner(object):
+    """Run a sequence of SIPp cmds asynchronously. If any process terminates
+    with a non-zero exit code, immediately canacel all remaining processes and


typo, should be cancel.

goodboy · 2020-12-24T13:37:43Z

pysipp/launch.py

        # run agent commands in sequence
        for cmd in cmds:
            log.debug(
                "launching cmd:\n\"{}\"\n".format(cmd))
-            proc = sp.Popen(
+            # proc = await trio.open_process(
+            proc = trio.Process(


This api is now old.

We should probably port to the newer trio.open_process().

goodboy · 2020-12-24T13:43:24Z

pysipp/launch.py

-                proc.streams = Streams(*proc.communicate())  # timeout=2))
-                if proc.returncode != 0 and not signalled:
+
+        # taken mostly verbatim from ``trio.run_process()``


There's also new work in python-trio/trio#1568 that will likely make some of this simpler.

kontsaki · 2020-12-24T16:06:12Z

@goodboy want me to open a PR for this branch if i get the chance and fix what you have commented?

goodboy · 2021-01-08T23:53:04Z

pysipp/launch.py

+                proc.stderr_output = await read_output(proc.stderr)
+
+        try:
+            with trio.fail_after(timeout):


@kontsaki yeah i'm thinking we should maybe factor this out just so it's clear that it's a global timeout?

Drop py27 more

goodboy · 2021-01-14T16:31:58Z

@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.

goodboy · 2021-02-13T17:31:55Z

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.

kontsaki · 2021-02-13T17:47:54Z

i will fix the conflicts

goodboy · 2021-02-13T18:20:27Z

@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.

merge master on drop_py27

goodboy · 2021-05-28T18:56:00Z

@kontsaki ok ur black changes should be in here now so let's take a look at #65 🥳

goodboy · 2021-05-28T19:07:11Z

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.

goodboy · 2021-05-28T19:11:36Z

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.

goodboy · 2021-05-28T19:19:57Z

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 😉

goodboy · 2021-05-28T19:44:31Z

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.

goodboy · 2021-09-15T16:55:55Z

@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:

on using an explicit .run() api vs. .call()`, and here as well.
back and forth on whether exposing the runner to user code is a good idea.

goodboy · 2021-09-15T16:56:50Z

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.

linuxmaniac · 2022-12-05T07:10:13Z

@goodboy are you willing to rebase this PR?

goodboy · 2022-12-18T20:48:31Z

@linuxmaniac yup just put up a redo with all merge conflicts fixed in #87

goodboy added 11 commits July 5, 2019 12:24

Add some additional logdir passing tests

701dca4

Relates to #20

Add official release stamp and be looser on pluggy version

c3d6bbd

Raise ValueError on non-tuple assignment

c275334

These fancy tuple-attribute-properties are supposed to be strictly assigned tuple types; let's enforce that. Resolves #12

Doc tweak

7544b5d

Use latest official sipp release in CI

8ecbdb0

Moderninze setup.py to new PyPa standards

a41df32

Drop runner related hooks

1ecc513

Adjust tests to match new trio apis

de44f50

goodboy requested review from vodik and wdoekes July 11, 2019 02:31

Drop py2.7 from CI

3911de9

Prepare setup script for 1.0 release

d11fe44

goodboy force-pushed the drop_py27 branch from 0fe5319 to a0a57bd Compare July 12, 2019 21:35

trio.open_process() isn't released yet

53bf511

goodboy force-pushed the drop_py27 branch from a0a57bd to 53bf511 Compare July 12, 2019 21:41

goodboy force-pushed the 0.1.0_release branch from a41df32 to 64850d1 Compare July 27, 2019 15:19

goodboy mentioned this pull request May 26, 2020

[pysipp] Scenario - Connection Error: socket already in use #59

Open

goodboy commented Dec 24, 2020

View reviewed changes

goodboy mentioned this pull request Dec 24, 2020

OS support, Windows and OS X when? #60

Open

Base automatically changed from 0.1.0_release to master December 24, 2020 14:54

kontsaki added 4 commits December 26, 2020 10:25

Fix running agents with trio

77cf11a

Merge remote-tracking branch 'upstream/drop_py27' into drop_py27

576b5be

Merge branch 'drop_py27' into drop_py27_more

e58c5ed

Fix ScenarioType uninitialized attribute

2d30b87

goodboy commented Jan 8, 2021

View reviewed changes

goodboy mentioned this pull request Jan 9, 2021

Drop py27 more #63

Merged

Merge pull request #63 from kontsaki/drop_py27_more

50cf7a1

Drop py27 more

goodboy added 2 commits February 12, 2021 10:39

Be explicit on dev version

403c46e

Update readme to reflect python version requirement

ab27aa5

goodboy force-pushed the drop_py27 branch from b8631dd to ab27aa5 Compare February 13, 2021 16:58

Merge branch 'master' into drop_py27

87f8efc

kontsaki mentioned this pull request Mar 22, 2021

merge master on drop_py27 #69

Merged

Merge pull request #69 from kontsaki/drop_py27

acf8374

merge master on drop_py27

goodboy mentioned this pull request May 28, 2021

Make scenario finalizing more functional #65

Closed

goodboy mentioned this pull request Dec 18, 2022

trio subprocs launching #87

Open

4 tasks

goodboy closed this Dec 18, 2022

goodboy mentioned this pull request Mar 23, 2023

Any chance to get a 0.2.0 version? #90

Open

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Drop py2.7 and add trio support! #55

Drop py2.7 and add trio support! #55

goodboy commented Jul 11, 2019

goodboy commented Jul 11, 2019

goodboy commented Jul 12, 2019

kontsaki commented Dec 24, 2020

goodboy commented Dec 24, 2020

goodboy Dec 24, 2020

goodboy Dec 24, 2020

goodboy Dec 24, 2020

kontsaki commented Dec 24, 2020

goodboy Jan 8, 2021

goodboy commented Jan 14, 2021

goodboy commented Feb 13, 2021

kontsaki commented Feb 13, 2021

goodboy commented Feb 13, 2021

goodboy commented May 28, 2021

goodboy commented May 28, 2021 •

edited

Loading

goodboy commented May 28, 2021 •

edited

Loading

goodboy commented May 28, 2021

goodboy commented May 28, 2021

goodboy commented Sep 15, 2021

goodboy commented Sep 15, 2021

linuxmaniac commented Dec 5, 2022

goodboy commented Dec 18, 2022

Drop py2.7 and add trio support! #55

Drop py2.7 and add trio support! #55

Conversation

goodboy commented Jul 11, 2019

goodboy commented Jul 11, 2019

goodboy commented Jul 12, 2019

kontsaki commented Dec 24, 2020

goodboy commented Dec 24, 2020

goodboy Dec 24, 2020

Choose a reason for hiding this comment

goodboy Dec 24, 2020

Choose a reason for hiding this comment

goodboy Dec 24, 2020

Choose a reason for hiding this comment

kontsaki commented Dec 24, 2020

goodboy Jan 8, 2021

Choose a reason for hiding this comment

goodboy commented Jan 14, 2021

goodboy commented Feb 13, 2021

kontsaki commented Feb 13, 2021

goodboy commented Feb 13, 2021

goodboy commented May 28, 2021

goodboy commented May 28, 2021 • edited Loading

goodboy commented May 28, 2021 • edited Loading

goodboy commented May 28, 2021

goodboy commented May 28, 2021

goodboy commented Sep 15, 2021

goodboy commented Sep 15, 2021

linuxmaniac commented Dec 5, 2022

goodboy commented Dec 18, 2022

goodboy commented May 28, 2021 •

edited

Loading

goodboy commented May 28, 2021 •

edited

Loading