Skip to content

Commit

Permalink
Add comments, pydocs, and asserts to better explain how ports work.
Browse files Browse the repository at this point in the history
  • Loading branch information
tvst committed Dec 4, 2018
1 parent 4b9e691 commit d7fb994
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 3 deletions.
5 changes: 4 additions & 1 deletion frontend/src/StreamlitApp.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,10 @@ class StreamlitApp extends PureComponent {
const reportName = query.name;
this.setReportName(reportName);

const port = IS_DEV_ENV ? WEBSOCKET_PORT_DEV : window.location.port;
// If dev, always connect to 8501, since window.location.port is the Node
// server's port 3000.
// If changed, also change config.py
const port = IS_DEV_ENV ? WEBSOCKET_PORT_DEV : +window.location.port;

const uri = getWsUrl(
window.location.hostname, port, reportName);
Expand Down
14 changes: 14 additions & 0 deletions lib/streamlit/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,12 @@ def _proxy_browser_port():
Default: 8501, but gets overriden by proxy.browserPortRange, if set.
"""
# When using the Node server, always connect to 8501. This is hard-coded in
# JS as well. Otherwise, the browser would decide what port to connect to
# based on either:
# 1. window.location.port, which in dev is going to be (3000)
# 2. the proxyPort value in manifest.json, which only exists when
# proxy.liveSave is true and the page is being served from storage.
if get_option('proxy.useNode'):
# IMPORTANT: If changed, also change baseconsts.js and StreamlitApp.js
return DEFAULT_BROWSER_PROXY_PORT
Expand Down Expand Up @@ -629,6 +635,14 @@ def _check_conflicts():
assert not (proxyPortManuallySet and portRangeManuallySet), (
'You cannot set both proxy.browserPort and proxy.browserPortRange')

assert not (proxyPortManuallySet and get_option('proxy.useNode')), (
'proxy.browserPort does not work in dev mode. '
'See comment in config._proxy_browser_port()')

assert not (portRangeManuallySet and get_option('proxy.useNode')), (
'proxy.browserPortRange does not work in dev mode. '
'See comment in config._proxy_browser_port()')


def _clean_paragraphs(txt):
paragraphs = txt.split('\n\n')
Expand Down
14 changes: 12 additions & 2 deletions lib/streamlit/proxy/ProxyConnection.py
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,9 @@ def _build_manifest(
configuredProxyAddress=configured_proxy_address,
externalProxyIP=external_proxy_ip,
internalProxyIP=internal_proxy_ip,
# Don't use _get_browser_address_bar_port() here, since we want the
# websocket port, not the web server port. (These are the same in
# prod, but different in dev)
proxyPort=config.get_option('browser.proxyPort'),
)

Expand All @@ -310,13 +313,20 @@ def _get_report_url(host, name):
The report's URL.
"""
port = _get_proxy_browser_port()
port = _get_browser_address_bar_port()

quoted_name = urllib.parse.quote_plus(name)
return 'http://{}:{}/?name={}'.format(host, port, quoted_name)


def _get_proxy_browser_port():
def _get_browser_address_bar_port():
"""Get the report URL that will be shown in the browser's address bar.
That is, this is the port where static assets will be served from. In dev,
this is different from the URL that will be used to connect to the
proxy-browser websocket.
"""
if config.get_option('proxy.useNode'):
return 3000
return config.get_option('browser.proxyPort')

0 comments on commit d7fb994

Please sign in to comment.