Dont start http server in Scheduler.__init__ #4896
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This needs much more work. Ran the wrong tests locally :/ |
|
I was curious what was going to happen there. I remember having to put the
HTTPServer in init for some reason. I was happy to see your initial PR,
but not surprised to see it closed ;(
…On Wed, Jun 9, 2021 at 10:36 AM Florian Jetter ***@***.***> wrote:
This needs much more work. Ran the wrong tests locally :/
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#4896 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACKZTCKGQGTOKUB53MC7BLTR6DBPANCNFSM46MECLBA>
.
|
|
One reason why it is in init is that we can then align security and ensure that if either scheduler comm or http server is secured, the other one must be as well. At least this is one not-trivial thing I found locally. Why the CI deadlocks immediately is beyond my understanding atm |
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This refactors the scheduler init such that the HTTP server is only started once the server is actually up. Otherwise every init call starts an http server and binds sockets, etc.
I tried testing for leaking sockets but that's not easy. Depending on OS this requires root and even with root, I had trouble asserting this properly using psutil.
Closes #4806