Skip to content

LatentWorker do not insubstantiate at shutdown#2317

Merged
tardyp merged 2 commits intobuildbot:masterfrom
tardyp:latent
Jul 15, 2016
Merged

LatentWorker do not insubstantiate at shutdown#2317
tardyp merged 2 commits intobuildbot:masterfrom
tardyp:latent

Conversation

@tardyp
Copy link
Copy Markdown
Member

@tardyp tardyp commented Jul 13, 2016

we already stop the workers at stopService, which is called by the Application's hierarchy stop

Doing the complicated SystemEventTrigger is just uneeded duplication

we already stop the workers at stopService, which is called by the Application's hierarchy stop

Doing the complicated SystemEventTrigger is just uneeded duplication
@mention-bot
Copy link
Copy Markdown

@tardyp, thanks for your PR! By analyzing the annotation information on this pull request, we identified @tomprince, @rutsky and @benallard to be potential reviewers

if self._substantiation_notifier:
# if master is stopping, we will never achieve consistent state, as workermanager
# wont accept new connection
if self._substantiation_notifier and self.master.running:
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@tardyp
Copy link
Copy Markdown
Member Author

tardyp commented Jul 13, 2016

Note that those fixes are not unit tested in this PR. They are tested in the tests for hyper.
I still need to write the tests specific for the core

from email.message import Message
from email.utils import formatdate

from future.utils import itervalues
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.

this import order change seems to be unnecessary

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

its made automatically by isort.

For some reason isort does not run on travis as it should, so we start taking technical debt about it.

That said. I would expect that future is part of the first. So maybe it is the config of my editor which cannot find the isort configuration

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

ahah I did not see the build failure. So isort travis definitly work and my editor is misconfigured :-p

@codecov-io
Copy link
Copy Markdown

Current coverage is 85.95%

Merging #2317 into master will increase coverage by <.01%

@@             master      #2317   diff @@
==========================================
  Files           293        293          
  Lines         30455      30445    -10   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits          26175      26168     -7   
+ Misses         4280       4277     -3   
  Partials          0          0          

Sunburst

Powered by Codecov. Last updated by a624266...3ecbd1d

@tardyp tardyp merged commit 588f598 into buildbot:master Jul 15, 2016
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.

4 participants