Move some functions from Server to BaseServer and BokehTornado #10070
Conversation
This file contains 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
bryevdv
force-pushed
the
bryanv/9183_base_server
branch
from
fe4a227
to
c20c0c8
Compare
2 tasks
bryevdv
changed the title
bryevdv
commented
May 25, 2020
|
|
||
| ''' | ||
| return self._prefix | ||
|
|
||
| @property | ||
| def port(self): |
There was a problem hiding this comment.
The one thing I don't love about this work is leaving the port and address properties instead of deferring to the ones on BaseServer. But if those are are used the log ouptut looks like:
2020-05-25 12:49:11,188 Bokeh app running at: http://0.0.0.0:57641/
instead of
2020-05-25 12:49:11,188 Bokeh app running at: http://localhost:57641/
So I think this will have to do for now.
bryevdv
commented
May 25, 2020
| @@ -170,7 +170,7 @@ def test_prefix(ManagedServerLoop) -> None: | |||
| assert server.prefix == "" | |||
|
|
|||
| with ManagedServerLoop(application, prefix="foo") as server: | |||
| assert server.prefix == "foo" | |||
| assert server.prefix == "/foo" | |||
There was a problem hiding this comment.
I think this is actually more correct. Hopefully does not bite anyone.
|
I'm pretty sure this affords what was it the other PR so I will go ahead and merge. |
|
Hey @bryevdv ! I apologize for going dark here. It's been a busy time and I'm just now going through some github e-mail. |
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.
@mrocklin In the interest of saving you some time I made a new PR with the additional changes needed to get tests passing. Can you test out this branch and verify that is satisfies your needs?