Skip to content

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 19 additions & 17 deletions robyn/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from robyn.argument_parser import Config
from robyn.authentication import AuthenticationHandler
from robyn.dependency_injection import DependencyMap
from robyn.dev_server import start_dev_server
from robyn.env_populator import load_vars
from robyn.events import Events
from robyn.jsonify import jsonify
Expand Down Expand Up @@ -56,8 +57,6 @@ def __init__(
"SERVER IS RUNNING IN VERBOSE/DEBUG MODE. Set --log-level to WARN to run in production mode.",
color=Colors.BLUE,
)
if self.config.dev:
exit("Dev mode is not supported in the python wrapper. Please use the CLI. e.g. python3 -m robyn app.py --dev ")

self.router = Router()
self.middleware_router = MiddlewareRouter()
Expand Down Expand Up @@ -225,21 +224,24 @@ def start(self, host: str = "127.0.0.1", port: int = 8080, _check_port: bool = T

mp.allow_connection_pickling()

run_processes(
host,
port,
self.directories,
self.request_headers,
self.router.get_routes(),
self.middleware_router.get_global_middlewares(),
self.middleware_router.get_route_middlewares(),
self.web_socket_router.get_routes(),
self.event_handlers,
self.config.workers,
self.config.processes,
self.response_headers,
open_browser,
)
if self.config.dev:
start_dev_server(self.config, self.config.file_path)
else:
run_processes(
host,
port,
self.directories,
self.request_headers,
self.router.get_routes(),
self.middleware_router.get_global_middlewares(),
self.middleware_router.get_route_middlewares(),
self.web_socket_router.get_routes(),
self.event_handlers,
self.config.workers,
self.config.processes,
self.response_headers,
open_browser,
)

def exception(self, exception_handler: Callable):
self.exception_handler = exception_handler
Expand Down
5 changes: 5 additions & 0 deletions robyn/argument_parser.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import argparse
import sys


class Config:
Expand Down Expand Up @@ -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]
Copy link

Choose a reason for hiding this comment

The 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.file_path = sys.argv[0] if len(sys.argv) > 0 else ''

📚 Relevant Docs


React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)

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")
Expand Down
19 changes: 4 additions & 15 deletions robyn/cli.py
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
Copy link

Choose a reason for hiding this comment

The 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.


React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)

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
Expand Down Expand Up @@ -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()
Expand Down
24 changes: 24 additions & 0 deletions robyn/dev_server.py
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):
"""
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()
Copy link

Choose a reason for hiding this comment

The 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.

📚 Relevant Docs


React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)

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
28 changes: 20 additions & 8 deletions robyn/reloader.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import os
import glob
import os
import signal
import subprocess
import sys
Expand All @@ -9,6 +9,7 @@
from watchdog.events import FileSystemEventHandler
from watchdog.observers import Observer

from robyn.argument_parser import Config
from robyn.logger import Colors, logger

dir_path = None
Expand Down Expand Up @@ -58,8 +59,8 @@ def clean_rust_binaries(rust_binaries: List[str]):
os.remove(file)


def setup_reloader(directory_path: str, file_path: str):
event_handler = EventHandler(file_path, directory_path)
def setup_reloader(config: Config, directory_path: str, file_path: str):
event_handler = EventHandler(config, file_path, directory_path)

# sets the IS_RELOADER_RUNNING environment variable to True
event_handler.reload()
Expand Down Expand Up @@ -92,7 +93,8 @@ def terminating_signal_handler(_sig, _frame):


class EventHandler(FileSystemEventHandler):
def __init__(self, file_path: str, directory_path: str) -> None:
def __init__(self, config: Config, file_path: str, directory_path: str) -> None:
self.config = config
self.file_path = file_path
self.directory_path = directory_path
self.process = None # Keep track of the subprocess
Expand Down Expand Up @@ -120,10 +122,20 @@ def reload(self):
if prev_process:
prev_process.kill()

self.process = subprocess.Popen(
[sys.executable, *arguments],
env=new_env,
)
if self.config.dev and self.config.running_as_module:
module_name = self.config.file_path.split("/")[-2]

self.process = subprocess.Popen(
[sys.executable, "-m", module_name, *arguments],
env=new_env,
start_new_session=False,
)
else:
self.process = subprocess.Popen(
[sys.executable, *arguments],
env=new_env,
start_new_session=False,
)

self.last_reload = time.time()

Expand Down
Loading