-
Notifications
You must be signed in to change notification settings - Fork 25
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
Hot module reloading #122
Comments
I tried to take a little look at this myself. What I found was that You might get some idea by looking at Genie's code: Happy to help test/review a PR. |
Hi @Dale-Black & @frankier Thanks for opening this ticket and for the suggestions. This is a good next issue to tackle which would definitely help speed up local development. I'll definitely try to follow what Genie.jl is doing to get this working, but I'm open to any and all suggestions. I did find a similar approach in this issue: JuliaWeb/HTTP.jl#587 Also I'll let you know when I have something worth testing & reviewing @frankier, thanks for the help! |
That http.jl code looks great. So something like HTTP.listen with Revise should work? |
Is there any progress in this direction? |
There was some progress made in a PR & discussion about a month ago - but I haven't worked on it since. I've been very busy this December. |
I use Revise and to do it I have: include("routes.jl")
__revise_mode__ = :eval
Revise.track("routes.jl")
function ReviseHandler(handle)
req -> begin
Revise.revise()
invokelatest(handle, req)
end
end |
Hmm, and that works? Can you share a bit more of your code and structure? Where are you serving the app from? |
Hi guys, Sorry about the inactivity on this issue. This issue is now on the top of my todo list. As far as direction and approaches, I'm all ears. Like most things, this is new territory for me and I'm open to any and all suggestions on how to tackle this problem. @asjir thanks for your snippet. Id also like to hear more about your project setup and workflow. I really like how you embedded this feature as a middleware function, which would be a super clean way to integrate behind the scenes |
Ofc, though it's very basic: in my repo I have sveltekit projects and one folder for julia server run(; port=2227, async=true) = (!isdefined(Main, :s) || !isopen(Main.s)) && (Main.s = serve(middleware=[CorsHandler, ReviseHandler]; port, async))
run() this function starts an async server that I can and inside include("util.jl")
Revise.track("util.jl") so changes to util are also immediately reflected. While I'm at it I'll show an example route: post("/make-qch") do req
data = json(req, MyStruct)
saved[] = data
f(data) # returns NamedTouple
end which allows me to call |
It looks like Fons' new project has built in hot module reloading. I haven't worked with it yet but something to look into |
I can report that I have had some success with
The
where the |
That looks clean |
The pull request is now merged on master. It would be great to get a feedback on the Revise workflow before the release. |
Amazing, works very nicely! (https://github.com/Dale-Black/HTMLStrings.jl/tree/main/examples/TodoApp) Now I have more motivation to keep exploring "full stack" Julia. Excited to build out a better TodoApp with pure Julia as an example |
@Dale-Black, is there any reason why you have chosen to register routes into the |
No there isn't. I just changed that |
I am following this with great interest. Unfortunately I cannot get the approach from #122 (comment) to work, possibly because I don't understand how to setup folder structures and modules properly. Specifically, I am doing:
and
Server works, but doesn't reload. What is strange, though, is that I need to restart Julia for changes to submodule.jl to register. A simple Ctrl-C and rerun of the server.jl (using run in REPL from in vscode) is not sufficient. Is it required to have ModuleA in a proper package that is added as a dev dependency? |
The approach I outlined earlier works only when the module is loaded statically, which Revise then picks up. To get the setup working, use |
Thanks. I got bit by not understanding modules vs packages and the functionality of I also managed to get it working using
The development then happens in this ServerPackage. Separate from this, a new server.jl resides in a project that has ServerPackage as a development dependency. (
And then, (just because it is possible), I run a client in separate process that repeatedly pings the API. |
I've also managed to get the Revise-before-service-request approach working. Thanks a lot! It is very nice in its simplicity, however, it is in a way a "just-too-late" approach. Revising can take time, and it moves the process from when the changes are ready in the source to when you want the page to be rendered, meaning longer waits/increase latency. I seem to be getting reasonably results with a combined approach where we have Revise watch the file system and runs asynchronously with the server process and then in-addition run Revise before service request. This, in combination with invokelatest appears to make the changes visible in the server thread. It does not appear to be strictly needed to also have Revise running in the server task, but I added this to try and ensure the request is blocked until revising is finished for longer revisions.
|
The asynchronous I see that it could be useful when developing a web server with live reloading. By sending a refresh event to the browser after the asynchronous revision, it could be a cool feature to have in development. |
Good catch! To be perfectly honest I wasn't even thinking about whether revision_event was edge or level triggered. This code was fairly carelessly cribbed from
Can you please explain this one for me a bit more? In case multi-threaded Revise is not possible, might we be able to force all the Oxygen.jl + Revise.jl code onto a single thread somehow?
This is even a further step of "live" but it's not clear when it's wanted and when not, so anything like this should be somehow "opt-in". Personally I'm already used to refreshing the page so don't condition it a problem. Bonito's |
Another point of information is that just-in-time (or just-too-late) Revise is what is used for REPLs. See: So maybe it's not such a problem after all. I may be having getting slow Revise on 1.11rc2 for some reason or it might just be that a slow loading webpage is somehow a lot more irritating than pending output on a REPL. |
I don't think thread safety is a problem in practice here since the file watching tasks run in the main thread (they are started as sticky Tasks) which is the same as the file-change trigger Revise loop. I'm probably making the race condition worse by having |
The thread safety simply means that the condition can be waited from another thread without risking putting internal state of the condition in inconsistent state. It doesn't fix race conditions when a Condition is notified from one thread while on other it is no longer suspended on wait and hence is skipped. This problem is not present when the notifier and waiter are on the same thread. I guess this is the case here if the task for watching file changes runs on the main thread or the notifications are channeled through one. |
I guess first entr(...) should be fixed first if this is a problem. See this issue timholy/Revise.jl#837 |
Any updates on this? Or is there any elegant way to perform some pseudo-hot-reloading (e.g. restart the server by an external script that monitor the git history) |
@mzy2240 It is, in my opinion, a solved issue, apart from proper documentation. Server restarts are unnecessary, but to use hot reloading, one must (a bit of a strong word here) develop their service in a package together with |
that sounds great! if that is the case, I have two sincere suggestions:
|
I don't entirely agree with this. It seems to me that if possible it should be possible to get good reloading behavior with a single function call, macro call or keyword option. We could try and design this so that it would only be usable in the case the user code was actually revisable (i.e. their project was correctly structured). I give another go at put together another pull request, but I would like to hear from @ndortega about whether they agree and what type of API they would like.
Why is this a better place to start than the lower latency version I added? The race condition is benign and exists also in Revise.jl. |
I don't really follow. From my perspective, the One thing that could though improve user experience is an automatic addition of Also, I forgot there should be a clear way to communicate to the user that Revises no longer works, for instance, when a type is redefined or a constant change is made. The server could exit, return some HTTP error code, or put some colour in the terminal logging as it is already in Julia REPL. |
A generic programmatic service seems to belong to a separate issue, but it is an interesting feature to ask for in the scope of this issue. Using Revise in production seems like a wild idea, but why not? A big issue is that restarts erase the internal service state. So, to implement service restarts, there would need to be some |
I really like the idea of persistent state but I think it should be page 2 or 3. Do we have an existing restart solution (scheduled or external triggered) even at the risk of losing state? That would be already quite useful in some simple but reliability-first applications. |
In that case I misunderstood you. It seems that we are probably agreed that this will be "solved issue" when it's actually solved by adding this or something similar to Oxygen (i.e. the issue is solved-in-principle).
In terms of user experience, a delay in a REPL due to compiling is more expected than a delay in page loading, which has a significantly less responsive feel. Browsers can actually time out too I think for an especially long revise. Another alternative or parallel solution could be to push some kind of spinner to the browser due to revising and refresh the page when it's done -- making sure to preserve whatever POST/GET data from the request.
Maybe I'm misunderstanding you again. Which race condition are you referring to? That the revise event can be lost in case there are changes during a revision? This isn't a big problem since it will get revised later before the request. Maybe you can outline the exact scenario where you foresee problems.
I agree that it would be a better user experience for those hacking and testing against a browser for some stuff to be pushed "in-band", but this is an argument for a special |
With the race condition, I meant the effect of the Revise limitation of being unable to cancel running recompilation. Let's say
It would be better to make APIs deep by adding a keyword argument. Also, it would be more intuitive for the user if Oxygen recognised that Revise is loaded and added the handler accordingly. |
I'm not entirely sure which approach would be best. At the end of the day, I think Oxygen needs auto-reload feature that works out of the box with as little "gotchas" as possible. I personally don't use Revise.jl all that much, but I know a huge portion of the julia community does and has great success with it. I'm definitely open to either adding it as a dependency / extension to get this feature working. On the other hand, most other frameworks do complete server restarts to ensure the current running application is up to date. This has been a tried-and-true method we could use to solve this problem as well. I haven't used Genie in a while, but I know they have a similar feature where they use Revise to watch all project files and are able to upload the handlers in real time. Granted, it's a lot easier to do this when you have a predefined folder structure to monitor. This package enforces near-zero architectural restrictions on where code comes from so we can't make as many assumptions about where their code will be stored. I'm not sure which approach would be best, so I'd like to hear more from you all on which approach you'd prefer based on your experience you have with other web frameworks) |
It is my guess that the restarting function would need to contain
Genie had to bolt in Revise with a watcher as it cannot develop the code within a package structure due to globals being erased at compilation time, much like Oxygen before the
What you may be able to do is to create a sandbox module within |
In terms of "complete restart", it may be possible to replace the process with a whole new Julia process with I'm not sure if there's perfect behavior here, more like a convenience which helps improve coding flow a bit 90% of the time, which is why I'm tempted to think Revise is the sweet spot despite a few caveats, especially since it's behaviour is reasonably well understood within the broader Julia community. My preference would therefore be to have a debug server mode that is either only available when |
Shall I put together another PR as a strawman, or does someone else want to? It feels like that might get us further at this point. (Asking because @ndortega was the last to express interest.) |
I would be in favour of implicit behavior using Revise loading in the main as condition under which the handler is added. This condition could be explicitly disabled with keyword argument when needed. A good way to do this seems to be is to check |
@frankier, please feel free to open up a pr here. I agree that going with a Revise-first approach makes the most sense right now - if it really isn't sufficient for most workflows then we could look into adding in a server-restart later. Personally, I'm interested in getting this functionality added directly to the code (instead of through a Revise package extension) so that people could toggle the revision of their code out of the box with a Whatever the implementation looks like, having a clean way to toggle this behavior is my biggest priority. So far, I've avoided building in this direction because my early attempts felt super hacky and required way too much work from the user's perspective. |
Is there a way to restart the server on file changes within the directory?
I was thinking something like BetterFileWatcher.jl might be useful for something like this but I'm not sure how to implement it
The text was updated successfully, but these errors were encountered: