-
-
Notifications
You must be signed in to change notification settings - Fork 290
feat: live reloading when running as a module #858
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
base: main
Are you sure you want to change the base?
Changes from all commits
78b9dda
6cab870
7f47788
8b06cdd
63970c1
c72e01a
933eddf
a7272e5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
import argparse | ||
import sys | ||
|
||
|
||
class Config: | ||
|
@@ -81,12 +82,16 @@ def __init__(self) -> None: | |
self.version = args.version | ||
self.compile_rust_path = args.compile_rust_path | ||
self.create_rust_file = args.create_rust_file | ||
self.running_as_module = False | ||
|
||
# find something that ends with .py in unknown_args | ||
for arg in unknown_args: | ||
if arg.endswith(".py"): | ||
self.file_path = arg | ||
break | ||
else: | ||
self.file_path = sys.argv[0] | ||
VishnuSanal marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unsafe access to sys.argv[0] without checking if sys.argv is empty. This can raise an IndexError when Python is run in environments where sys.argv is an empty list (e.g., certain Python interpreters or when running through specific APIs). The code should check sys.argv length before accessing the first element. A safer approach would be:
|
||
self.running_as_module = True | ||
|
||
if self.dev and (self.processes != 1 or self.workers != 1): | ||
raise Exception("--processes and --workers shouldn't be used with --dev") | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,12 @@ | ||
import os | ||
import sys | ||
from typing import Optional | ||
import webbrowser | ||
from InquirerPy.resolver import prompt | ||
from InquirerPy.base.control import Choice | ||
from .argument_parser import Config | ||
from .reloader import create_rust_file, setup_reloader | ||
|
||
from robyn.dev_server import start_dev_server | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The import changes introduce a breaking change in dependency initialization. The old code imported and used setup_reloader directly from reloader.py, but the new dev_server.py only imports the reloader module without importing setup_reloader. This will cause a NameError when the dev server tries to use setup_reloader since it's not properly imported in dev_server.py.
|
||
from robyn.argument_parser import Config | ||
from robyn.reloader import create_rust_file | ||
from robyn.robyn import get_version | ||
from pathlib import Path | ||
import shutil | ||
|
@@ -76,18 +77,6 @@ def docs(): | |
webbrowser.open("https://robyn.tech") | ||
|
||
|
||
def start_dev_server(config: Config, file_path: Optional[str] = None): | ||
if file_path is None: | ||
return | ||
|
||
absolute_file_path = (Path.cwd() / file_path).resolve() | ||
directory_path = absolute_file_path.parent | ||
|
||
if config.dev and not os.environ.get("IS_RELOADER_RUNNING", False): | ||
setup_reloader(str(directory_path), str(absolute_file_path)) | ||
return | ||
|
||
|
||
def start_app_normally(config: Config): | ||
# Parsing the known and unknown arguments | ||
known_arguments, unknown_args = config.parser.parse_known_args() | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
import os | ||
from pathlib import Path | ||
from typing import Optional | ||
|
||
from robyn import reloader | ||
from robyn.argument_parser import Config | ||
|
||
|
||
def start_dev_server(config: Config, file_path: Optional[str] = None): | ||
VishnuSanal marked this conversation as resolved.
Show resolved
Hide resolved
|
||
""" | ||
Start the dev server. Initialize the reloader to monitor file changes | ||
|
||
@param config: the config object | ||
@param file_path: the path to the file | ||
""" | ||
if file_path is None: | ||
return | ||
|
||
absolute_file_path = (Path.cwd() / file_path).resolve() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unsafe path resolution: Using resolve() on a potentially non-existent path without error handling can raise FileNotFoundError or other path-related exceptions. This could crash the server when given invalid paths. Should wrap the path resolution in a try-catch block to handle potential path-related exceptions gracefully.
|
||
directory_path = absolute_file_path.parent | ||
|
||
if config.dev and not os.environ.get("IS_RELOADER_RUNNING", False): | ||
reloader.setup_reloader(config, str(directory_path), str(absolute_file_path)) | ||
return |
Uh oh!
There was an error while loading. Please reload this page.