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

add Exception type for MultiError.catch #630

Closed
wants to merge 2 commits into from
Closed

add Exception type for MultiError.catch #630

wants to merge 2 commits into from

Conversation

WindSoilder
Copy link
Contributor

which added for feature #408 . And change some relative unit test

@njsmith
Copy link
Member

njsmith commented Aug 24, 2018

Hey, thanks for jumping in to work on this!

Implementing the ideas in #611 is going to be a bit complicated. We're going to have to make several compatibility-breaking changes:

  • nurseries start raising MultiErrors, even if there's only one exception
  • MultiError.catch starts taking the exception type (as in this PR)
  • MultiError.catch starts calling the handler once with a combined exception, instead of a bunch of times on individual exceptions

Also it'd be nice to coordinate a bit with the asyncio folks before committing to something here, since we're hoping to converge with them on this part.

Given all that, I think we probably want to move a little slowly here, and try to batch up all these changes together, instead of doing multiple rounds of compatibility-breaking changes. So, I'm afraid I'm rejecting this PR in its current form.

#607 is somewhat similar in terms of refactoring some of trio's core APIs, and there's a pretty concrete plan for what to do (especially in this comment: #607 (comment), once you dig through all my verbiage). Or if you want to keep working on the MultiError stuff, I have a half-written prototype locally that I'll try to at least push up somewhere in the next few days, and we could work together on that?

@njsmith njsmith closed this Aug 24, 2018
@WindSoilder
Copy link
Contributor Author

WindSoilder commented Aug 27, 2018

Hi, Sorry for the late reply..
It's very great for me to keep working on the MultiError stuff.

And, of cause I can also try to help on #607 when I have time :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants