Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: merge pytest-trio into the main Trio package #79

Closed
Zac-HD opened this issue Feb 4, 2019 · 10 comments

Comments

@Zac-HD
Copy link
Member

commented Feb 4, 2019

Installing compatible versions of pytest-trio and Trio itself is unreasonably difficult. pytest-trio often emits warnings on the latest version of Trio, but is broken on much older versions - though we don't specify a minimum version of Trio in the install_requires.

Fortunately, there is a better way: we can simply distribute the pytest plugin as an integral part of Trio.

  • No runtime impact, unless of course you run pytest
  • Not a novel strategy (Hypothesis has been doing this for years)
  • No need to coordinate releases, or make changes across two repos

Doing some Git magic would even let us graft in all the history from this repo. Finally, we'd finish by making a last release of pytest-trio that marks the package as inactive, and does gives an import-time error if used with a post-merge version of Trio.

Thoughts? Particularly interested in hearing from @njsmith and @pquentin before I look at making some more PRs 😄

@njsmith

This comment has been minimized.

Copy link
Member

commented Feb 4, 2019

(Is pytest-trio spitting out any warnings currently that we need to fix?)

Trio itself is still in the adolescent stage where it's fairly aggressive about deprecating and changing things. This does make for churn across projects that use trio, and pytest-trio is certainly a project that uses trio :-). I don't know that it's any worse than other trio-based projects like trio-asyncio, trio-websockets, etc., though? pytest-trio doesn't use any internal APIs or anything. This is one reason why it would be really nice to have better release tooling, so we can more easily fix things up after Trio deprecates something... (python-trio/trio#220)

I've generally tried to be pretty mindful about splitting things up into sensibly scoped projects, on the theory that (a) the trio ecosystem is eventually going to be pretty big and diverse, so we might as well figure out how to do that right from the beginning, (b) my experience with big foundational projects like numpy or twisted is that it's very easy to end up with a lot of tenously-related stuff in your project that eventually turn into millstones around the necks of maintainers. A few years ago, the question here would be about merging nose-trio into the main Trio package, and I bet we'd be regretting that today ;-).

I'm not saying this is definitely a bad idea, just giving some background reasoning.

@Zac-HD

This comment has been minimized.

Copy link
Member Author

commented Feb 4, 2019

(Is pytest-trio spitting out any warnings currently that we need to fix?)

Yep, about CancelScope. I've just bundled that with #73, but of course CI won't pass until we release a new version of Trio! (maybe we could also adopt Hypothesis' release-every-PR tooling?)

I don't know that it's any worse than other trio-based projects like trio-asyncio, trio-websockets, etc.? ... it's very easy to end up with a lot of tenously-related stuff in your project that eventually turn into millstones around the necks of maintainers

Well, I've spent less time working on those, so... 🤣

More seriously, I'd personally integrate them too, at least once they're stable and going to be useful for the foreseeable future (so pytest-trio yes; hypothesis-trio no). For now, the alternative is to have the same maintainers working on all the projects, but with the added burden of cross-repo coordination!

My experience with Hypothesis is that bringing subprojects into a single parent package is a substantial net win, and that it's not that hard to deprecate things if we really need to - we're nowhere near the age or complexity of Twisted or Numpy yet, and I think we'll have a lot of warning before we get close.

I can see the argument against merging, but my experience with OSS monorepos has been good.

@pquentin

This comment has been minimized.

Copy link
Member

commented Feb 5, 2019

I would also prefer to keep the repositories splitted, but I've never released one so I don't know how painful it is!

I think the main difference with Hypothesis is that the total scope of Hypothesis is narrower, so including everything makes more sense.

@Zac-HD

This comment has been minimized.

Copy link
Member Author

commented Feb 6, 2019

Maybe just automating the release process then?

That's also something we've done in Hypothesis, and per the article above it's amazing to work with. It doesn't need much code either; I wrote a travisCI one-liner to automatically release every commit on master with a new version number. Hypothesis uses some more scripts to avoid version collisions from concurrent PRs, but it's not that complicated 😄

(for example: #73 is currently blocked waiting for a new release of Trio)

@pquentin

This comment has been minimized.

Copy link
Member

commented Feb 7, 2019

See python-trio/trio#220 for a discussion about automating the trio release process. (Well, you've participated there already 😄 but I thought the crosslink was useful.)

@smurfix

This comment has been minimized.

Copy link
Contributor

commented Feb 13, 2019

I'm in favor of merging because the time between a Trio release and the corresponding pytest-trio upgrade results in a lot of third-party failures.

Right now, many tests for trio-asyncio fail because they complain about deprecations, and the latest release of pytest-trio still uses open_cancel_scope. Owch. Even worse, the actual location of the call to o_c_s doesn't even show up in the backtraces!

Trio could mitigate this by using a two-step deprecation model … but I consider that to be a workaround that can (and probably should) be avoided.

@njsmith

This comment has been minimized.

Copy link
Member

commented Feb 13, 2019

Trio could mitigate this by using a two-step deprecation model … but I consider that to be a workaround that can (and probably should) be avoided.

What's a "two-step deprecation model"?

@njsmith

This comment has been minimized.

Copy link
Member

commented Feb 13, 2019

okay, I released

@smurfix

This comment has been minimized.

Copy link
Contributor

commented Feb 13, 2019

What's a "two-step deprecation model"?

You first use a PendingDeprecation which by default doesn't emit a warning, then convert that to a "real" DeprecationWarning some time later.

Python itself does this.

@Zac-HD

This comment has been minimized.

Copy link
Member Author

commented Aug 19, 2019

Closing this as it's clear that we don't have consensus that merging the repos is a good idea, or any work ongoing that suggests it will happen in the foreseeable future.

@Zac-HD Zac-HD closed this Aug 19, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.