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
Add option to specify port #192
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making this change!
pynecone/utils.py
Outdated
@@ -533,6 +537,13 @@ def get_num_workers() -> int: | |||
return (os.cpu_count() or 1) * 2 + 1 | |||
|
|||
|
|||
def get_api_port(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs a return type annotation -> int
as well as a docstring to make the build happy.
The docstring can just be
"""Get the API port.
Returns:
The API port.
"""
pynecone/utils.py
Outdated
def get_api_port(): | ||
result = urlparse(get_config().api_url) | ||
if result.port is None: | ||
return urlparse(constants.API_URL).port |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may need to add an assert here for Pyright
port = urlparse(...)
assert port is not None
return port
pynecone/utils.py
Outdated
@@ -559,6 +571,8 @@ def run_backend_prod( | |||
""" | |||
num_workers = get_num_workers() | |||
command = constants.RUN_BACKEND_PROD + [ | |||
"--bind", | |||
"0.0.0.0:%s"%get_api_port(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use an f-string here for consistency with the rest of the code?
f"0.0.0.0:{get_api_port()}"
Yes thanks for implementing this feature definitely needed! 🙂 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me - it just needs to be linted. You can run
poetry run black pynecone
and it should format the code for you, then we can merge this
Done. @picklelo |
fix issue #157