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

Return runner's defer in Application.run #603

Merged
merged 1 commit into from
Feb 18, 2016

Conversation

touilleMan
Copy link
Contributor

Hi,

I'm working on a lib to easily integrate autobahn in a synchronous application (see https://github.com/Scille/autobahn_sync)
Given I use crochet, the twisted reactor is already started when I run Autobahn.Application, which means I have to handle by myself the connection errors (see https://github.com/Scille/autobahn_sync/blob/53c7303a06cf53a783fabbc550b334d48d90d05a/autobahn_sync/core.py#L599)
This is fine except Autobahn.Application.run doesn't return the ApplicationRunner's defer.
To fix that I had to copy the Application.run function inside my app (see https://github.com/Scille/autobahn_sync/blob/master/autobahn_sync/core.py#L59), which feels like adding complexity for nothing...

@codecov-io
Copy link

Current coverage is 57.39%

Merging #603 into master will not affect coverage as of 757e1b4

@@            master    #603   diff @@
======================================
  Files           61      61       
  Stmts        10229   10229       
  Branches      1662    1662       
  Methods          0       0       
======================================
  Hit           5871    5871       
  Partial        556     556       
  Missed        3802    3802       

Review entire Coverage Diff as of 757e1b4

Powered by Codecov. Updated on successful CI builds.

meejah added a commit that referenced this pull request Feb 18, 2016
Return runner's defer in Application.run
@meejah meejah merged commit 5438c0a into crossbario:master Feb 18, 2016
@meejah
Copy link
Contributor

meejah commented Feb 18, 2016

LGTM, thanks:)

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.

3 participants