Drop py27 more #63

kontsaki · 2020-12-26T09:27:12Z

@goodboy would something like this suffice ?

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

kontsaki · 2020-12-26T09:41:13Z

it seems that i haven't picked up the commit that resolved the conflicts even though i forked after that

kontsaki · 2021-01-05T20:13:55Z

ping @goodboy , anything else to do here?

goodboy · 2021-01-08T23:43:19Z

pysipp/agent.py

+        assert (
+            self._prepared_agents and self._runner
+        ), "Must run scenario before finalizing."
+        return trio.run(


Ahh right this is triggered from sync code yes?

Sorry trying to get my head back in this change set!

yes, it's not the most elegant solution, do you want me to try something that doesn't require abusing objects attributes and be more functional? (it may change the API a bit though)

If you have something functional that you think is cleaner then yeah for sure.

I have no real problems with this I just forgot this is how we have to do it.

maybe we can just return a handle to finalize from Scenario.run(), atm we return the runner which is exposing internals.

Yeah this is a good idea - feels like a continuation/future then.

goodboy · 2021-01-08T23:46:50Z

pysipp/launch.py

@@ -56,8 +56,7 @@ async def run(
        for cmd in cmds:
            log.debug(
                "launching cmd:\n\"{}\"\n".format(cmd))
-            # proc = await trio.open_process(
-            proc = trio.Process(
+            proc = await trio.open_process(


do we want the async context manager interface here?

I'm pretty sure there's some teardown we may find useful?

I'm not sure how this is going to change though given python-trio/trio#1568.

aren't changes in this trio PR going to make it so you can use trio.open_process inside a nursery?
in pysipp we want the Scenario API to not be async right?

do we want the async context manager interface here?

what do you have in mind? isn't the teardown part of finalize that will be called from sync code?

in pysipp we want the Scenario API to not be async right?

Right, I think that's what we have to aim for at the outset.

isn't the teardown part of finalize that will be called from sync code?

Yes you're right, so I guess I was thinking we should trigger the shutdown code I linked to, but because it's getting called from sync code we might have to either call Process.aclose() explicitly or we use an async exit stack. I'm thinking the former is simpler and cleaner?

goodboy · 2021-01-08T23:47:59Z

pysipp/launch.py

+async def finalize(runner, agents, timeout):
+    """Block up to `timeout` seconds for all agents to complete."""
+    # this might raise TimeoutError
+    cmds2procs = await runner.get(timeout=timeout)


Hmm could be maybe do this in more trionic style using with trio.fail_after(timeout): ?

Not sure what will have to get factored out of .get()

i will give it a try and see if we can refactor finalize() and .get()

Yah I mentioned the spot from my original branch here. Not sure if I'm convinced though given a lot of users might not be familiar with this style?

What do you think?

oh yes i would prefer a common global timeout, we can also document it

users might not be familiar with this style

which style are you referring to?

with trio.fail_after(1): scen.finalize()

I'm actually now thinking we can't do this anyway..
If we want to keep a sync api this won't work I don't think (fail_after() won't work outside a call to trio.run() pretty sure). I just don't have my head in this code so I'm probably making bad suggestions.

This should work if we offer an async .aclose() method to do finalization.

Oh wait, nm .get() is async now - see what I mean, obviously haven't looked at this in a while.

And, duh, we await that in the above code.
So yeah I think maybe pulling out the timeout is the more trionic way to do this thus giving the user more control on waiting / cancellation. The only down side is we'll have to document this style for async users.

goodboy

@kontsaki Thanks so much for this!

I think the one major thing we will need is a .dev0 bump to the setup.py and maybe we need to start a changelog?

goodboy · 2021-01-08T23:57:24Z

We might also need to tweak the docs which has some comments about the old runner.

goodboy · 2021-01-11T23:04:56Z

@kontsaki I think I might just merge this into #55 and we'll go off that branch.

Sound good?

kontsaki · 2021-01-12T06:32:21Z

@goodboy sure!

we can continue discussion there.

goodboy and others added 15 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

Drop py2.7 from CI

3911de9

Prepare setup script for 1.0 release

d11fe44

trio.open_process() isn't released yet

53bf511

Fix running agents with trio

77cf11a

kontsaki added 3 commits December 26, 2020 18:13

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 reviewed Jan 8, 2021

View reviewed changes

goodboy merged commit 50cf7a1 into SIPp:drop_py27 Jan 14, 2021

kontsaki deleted the drop_py27_more branch January 14, 2021 17:24

goodboy mentioned this pull request Mar 22, 2021

merge master on drop_py27 #69

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Drop py27 more #63

Drop py27 more #63

kontsaki commented Dec 26, 2020

kontsaki commented Dec 26, 2020

kontsaki commented Jan 5, 2021

goodboy Jan 8, 2021

kontsaki Jan 9, 2021

goodboy Jan 9, 2021

kontsaki Jan 12, 2021

goodboy Jan 14, 2021

goodboy Jan 8, 2021

kontsaki Jan 9, 2021

kontsaki Jan 9, 2021

goodboy Jan 9, 2021

goodboy Jan 8, 2021

kontsaki Jan 9, 2021

goodboy Jan 9, 2021

kontsaki Jan 9, 2021

goodboy Jan 9, 2021

goodboy Jan 9, 2021 •

edited

Loading

goodboy left a comment

goodboy commented Jan 8, 2021

goodboy commented Jan 11, 2021

kontsaki commented Jan 12, 2021

Drop py27 more #63

Drop py27 more #63

Conversation

kontsaki commented Dec 26, 2020

kontsaki commented Dec 26, 2020

kontsaki commented Jan 5, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

goodboy Jan 9, 2021 • edited Loading

Choose a reason for hiding this comment

goodboy left a comment

Choose a reason for hiding this comment

goodboy commented Jan 8, 2021

goodboy commented Jan 11, 2021

kontsaki commented Jan 12, 2021

goodboy Jan 9, 2021 •

edited

Loading