Skip to content
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

Load workers dynamically #175

Open
Angelmmiguel opened this issue Jul 11, 2023 · 3 comments
Open

Load workers dynamically #175

Angelmmiguel opened this issue Jul 11, 2023 · 3 comments
Assignees
Labels
🚀 enhancement New feature or request

Comments

@Angelmmiguel
Copy link
Contributor

Is your feature request related to a problem? Please describe.

When you start Wasm Workers Server, it retrieves all the workers in the given location (remote Git repo or local folder). After this initialization, it cannot add new workers on the fly and it requires you to restart the entire server.

Since we have an administration panel (#167), it would be nice to allow creating new workers on the fly or even edit existing ones. This is also related to the ability of "watching" running workers and reloading them (#157)

Describe the solution you'd like

I would like Wasm Workers Server to load new workers dynamically. Instead of a static list of Routes and a fixed set of URLs, the wasm_handler should receive any request that was not managed by other handler. Then, it iterates over the Routes and find the one that can reply it. The Routes struct must be mutable, so we can add new workers / routes under demand.

That approach opens new uses cases like the ones mentioned above.

Describe alternatives you've considered

No response

Additional context

No response

@Angelmmiguel Angelmmiguel added the 🚀 enhancement New feature or request label Jul 11, 2023
@Angelmmiguel Angelmmiguel self-assigned this Jul 11, 2023
@Angelmmiguel Angelmmiguel added this to the v1.6.0 milestone Sep 28, 2023
@Angelmmiguel
Copy link
Contributor Author

After starting working on this issue, I noticed some challenges here. Currently, we're registering specific routes for every worker and use the handle_worker method to reply. For example, we associate /demo from demo.js to handle_worker. However, we cannot follow this approach anymore as we may add workers lately.

Then, I added a default_service that captures any non-processed requests and pass them to handle_worker. The default service caused collisions with actix-files, which it's the library we're using to serve the static files. By default, wws mounts the assets in / which takes precedence and tries to reply to all requests.

For this reason, I had to invert the approach. Now, I plan to register all the static files in specific routes and then, use the handler_worker as the default_service. It works, although it's a bit tedious as know we need to manually iterate over the public folder.

All this complexity comes from the fact that we're also supporting "pretty" routes in the public folder. Like serving /public/index.html in /. If we don't support it, we can just associate any path that doesn't include a . character to the handle_worker method. Other option could be to mount the public folder in the /public prefix, although some specific static assets like robots.txt must be at the root level.

What do you think @ereslibre?

@ereslibre
Copy link
Contributor

ereslibre commented Sep 29, 2023

However, we cannot follow this approach anymore as we may add workers lately.

Why we cannot continue following this approach? Routes would be the discovered ones from the filesystem, plus the ones registered through the API. We might error out if an API call tries to register a path that clashes with an existing one, be it a discovered one or a previously registered path.

All this complexity comes from the fact that we're also supporting "pretty" routes in the public folder. Like serving /public/index.html in /. If we don't support it, we can just associate any path that doesn't include a . character to the handle_worker method. Other option could be to mount the public folder in the /public prefix, although some specific static assets like robots.txt must be at the root level.

If there is no other way, we can serve the public folder at /public, and visit later how to serve static assets in the root with the limitations you have described.

I think it would be great to have a diff to discuss on for being more specific.

@Angelmmiguel
Copy link
Contributor Author

Why we cannot continue following this approach? Routes would be the discovered ones from the filesystem, plus the ones registered through the API. We might error out if an API call tries to register a path that clashes with an existing one, be it a discovered one or a previously registered path.

The problem is the route collision. Currently, this is the chain we have:

1. Load workers:
  -> Register a route per worker
2. Mount actix-files on /
  -> It catches any route that wasn't previously defined

We cannot register new routes in actix after the server starts. For that, we need to rely on a default_handler that will reply to all the requests that are not processed by any other handler. However, actix-files is explicitly mounted, so it will catch all those cases first.

If there is no other way, we can serve the public folder at /public, and visit later how to serve static assets in the root with the limitations you have described.

This simplifies the approach, but limits the usability of the public resources as you cannot mount them on the root.

I think it would be great to have a diff to discuss on for being more specific.

Sure, let me open a draft and we can continue the discussion there 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🚀 enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants