-
-
Notifications
You must be signed in to change notification settings - Fork 252
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
fix: enable hot reload on windows #290
Conversation
✅ Deploy Preview for robyn canceled.
|
43c69ec
to
0db5952
Compare
Hi @guilefoylegaurav , thanks for your PR. I don't have an access to a windows machine till the 26th so can't really tell you about the windows issue. Dev server reflects the right changes on my osx machine but is not killed properly on KeyboardInterrupt. I will check my linux PC to give you updates about the debian state. |
robyn/dev_event_handler.py
Outdated
@@ -11,7 +12,8 @@ def __init__(self, file_name) -> None: | |||
def start_server_first_time(self) -> None: | |||
if self.processes: | |||
raise Exception("Something wrong with the server") | |||
self.processes.append(subprocess.Popen(["python3", self.file_name], start_new_session=False)) | |||
command = "python3" if not sys.platform.startswith("win32") else "python" |
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.
instead of this we can create a private computed python property that gives us a python alias depending on the platform. What do you think?
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.
Correct. That would be much better.
0db5952
to
98d33b3
Compare
@guilefoylegaurav , I think you pushed the same commit. |
Noticing that there were some updates to the upstream, I pulled the updates down from upstream to the fork's main and rebased the feature branch with the main and push that back, actually |
@guilefoylegaurav , are you working on converting duplicate code to a |
de09280
to
e6ab451
Compare
My apologies |
Just one last thing @guilefoylegaurav , have you run the https://github.com/sansyrox/robyn#%EF%B8%8F-to-develop-locally |
@sansyrox I was under the impression that the pre-commit runs automatically on git commit |
Ah, okay. This PR looks good. But I will do a final review tomorrow morning 😄 Also, the link that you shared is dead in the last comment. |
Thanks @sansyrox |
robyn/dev_event_handler.py
Outdated
@@ -7,11 +8,14 @@ class EventHandler(FileSystemEventHandler): | |||
def __init__(self, file_name) -> None: | |||
self.file_name = file_name | |||
self.processes = [] | |||
self.alias = "python3" if not sys.platform.startswith("win32") else "python" |
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.
One last nit: can you name it as python_alias
?
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.
Done
Not to worry 😄 Happens with all of us 😄 |
* fix(dev_event_handler.py): use platform specific python3 alias to spawn processes
e6ab451
to
2e4b8b9
Compare
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.
Great work! LGTM 🔥
* fix(dev_event_handler.py): use platform specific python3 alias to spawn processes
Description
Concerned with issue/bug #283
Changes
Documentation
No documentation update is required
How I've tested my work
global_index_request()
failed)Remarks
Although this minute code change gets the
dev_index_request()
test passed on windows, for some reason hot reload fails to work on my system.dev_event_handler.py
, functionon_any_event()
gets called every time I make a change in thebase_routes.py
file but I cannot see changes being reflected on the browser.dev_event_handler.py
, functionon_any_event()
does not get called when I make a change in thebase_routes.py
file and thus I cannot see changes being reflected on the browser.Failure of
global_index_request()
test persists