Skip to content

Add 'fork' to auto backend - #614

Merged
lovelydinosaur merged 2 commits into
masterfrom
add-fork-to-auto-backend
Dec 7, 2019
Merged

lovelydinosaur merged 2 commits into
masterfrom
add-fork-to-auto-backend

Conversation

@lovelydinosaur

Copy link
Copy Markdown
Member

Seems that we don't have coverage for this yet.

@florimondmanca florimondmanca left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, omission on my side! Thanks!

@lovelydinosaur

Copy link
Copy Markdown
Member Author

No sweat! 😃

@lovelydinosaur

Copy link
Copy Markdown
Member Author

Incidentally, #612 actually means we won't end up needing fork or a background manager at all.

@florimondmanca

Copy link
Copy Markdown
Contributor

Is this actually causing errors? Curious about why we didn't catch this… Probably worth refactoring our test setup to test all possible backends - right now I believe we force backend=AsyncioBackend() and backend=TrioBackend(), so don't cover the auto part nor passing a string.

@lovelydinosaur

Copy link
Copy Markdown
Member Author

Yes - causing a NotImplementedError when using HTTP/2.

We'll probably want to test using backend='auto', backend='trio', backend='asyncio', but there may be a bit of rejigging to do wrt. running the test server.

@lovelydinosaur
lovelydinosaur merged commit f55db15 into master Dec 7, 2019
@lovelydinosaur
lovelydinosaur deleted the add-fork-to-auto-backend branch December 7, 2019 09:07
@florimondmanca

Copy link
Copy Markdown
Contributor

@tomchristie I'm taking a look at it. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants