Repository navigation
Reconsider how we use/advertise idom.run() #657
Description
Activity
My personal opinion is that
idom.run()should either be removed or tucked away into a "dev tools" closet as-is and shouldn't be prominently advertised.Instead, we should focus on simplifying how users integrate IDOM into existing applications. It's an over-reach of scope for us to handle anything related to launching a WSGI/ASGI webserver.
- addedpriority-2-moderateShould be resolved on a reasonable timeline.Should be resolved on a reasonable timeline.priority-3-lowMay be resolved one any timeline.May be resolved one any timeline.type-revisionAbout a change in functionality or behaviorAbout a change in functionality or behavior
on Feb 8, 2022 We could add a big fat warning about this instead? Technically doing
idom.runis fine with Sanic (and I think Tornado too), but not the others.Meh, nevermind. We should completely shift how this works:
- Drop the run function
- stop calling things a per-client state "server" and instead call it a "view"
- Make
appa required argument ofPerClientStateViewetc. - Rename
idom.server.proto.ServerType.runtorun_dev_server
Doing this with appropriate deprecation warnings will be a bit tricky, but I think that sends a more accurate message about how you should expect to use IDOM.
The only question then is how you many the "hello world" still look simple...
Maybe do:
from idom import component, html, server @component def App(): return html.h1("Hello, World!") server.default.run(App)
Except
runonly accepts one argument (use lack of configuration as deterrent) and produces a big fat warning.or maybe
server.dev.run?- removedflag-triageNot prioritized.Not prioritized.priority-3-lowMay be resolved one any timeline.May be resolved one any timeline.
on Feb 8, 2022 There's also a question of the install target: #600
I think we should we remove any sense of a "default" install target too since you'd always need to explicitly call out which server implementation you're using.
There's no way to give a deprecation warning for this removal though as far as I can tell unless
pipknows to find the last version with a given install target.Doing this with appropriate deprecation warnings will be a bit tricky, but I think that sends a more accurate message about how you should expect to use IDOM.
IDOM is still
<1.0.0, technically we've never needed to deprecate anything. Immediate breaking changes are fully acceptable on pre-release version numbers.The only question then is how you many the "hello world" still look simple...
Maybe we consider a pattern like this (with the run callable name being TBD)
from idom import component, html, devtools from fastapi import FastAPI fastapi_app = FastAPI() @component def Hello(): return html.h1("Hello, World!") # This should print a warning that it is not production usable devtools.run(fastapi_app, component=Hello)
I'm worried about the hello-world implying a preference for a given server implementation.
That's pretty much an impossible problem to solve without multiple examples for each framework. We support multiple web frameworks, but we need to choose at least one to first demo with.
Could just be a comment saying "Utilize whatever Python web framework you like here!"
6 remaining items
I really dislike the name PerClientState
I've gotten an inverse comment at some point. With that said, I had a hard time coming up with a name that accurately described what they did so I'd be open to suggestions.
Just removing
Statefrom the name makes it a lot easier to understand. For examplePerClientViewandSharedView.On the topic of
SharedViewThe multiprocessing issue with
SharedViewwill become an even bigger problem as we start recommending people to use actual webservers to serve IDOM. I'm sure the majority of people use multiple worker processes in any realistic deployment scenario.I'd honestly just recommend removing it now. As far as I know, it's not possible to obtain a multiprocessing safe
SharedView. Especially in the context of a webserver's ASGI worker processes, since we don't have direct access to the worker process callable.Here's what I think all our options are
- Remove
SharedViewand have shared states to be left up to the user - Remove
SharedViewand create framework-specific shared state hooks when technologically possible - Have
redisbecome a requirement forSharedView
If we completely remove
SharedView, then we also gain the luxury of renamingPerClientViewto something simple likeIdomView.- Remove
On a side note, I've decided to commit to creating IDOM video tutorials. We're going to need something for our visual learners to increase adoption.
I'll try and make that happen sometime this year.
- addedtype-docsAbout changes and updates to documentationAbout changes and updates to documentation
on Feb 9, 2022 Let me know if you agree on #657 (comment)
- addedpriority-3-lowMay be resolved one any timeline.May be resolved one any timeline.and removedpriority-2-moderateShould be resolved on a reasonable timeline.Should be resolved on a reasonable timeline.
on Feb 9, 2022 I think this issue may be related: #653
Ah, whoops, didn't read through your comment.
Yes, I think I agree.
Probably with the addition that
idom.runtakes a single argument (the component) to discourage use in the long term.How do you feel about removing SharedView? #657 (comment)
I agree
Current Situation
idom.run()probably shouldn't be the primary advertised way of using IDOM. Especially in it's current state.Using a development webserver for production use is a big no-no, and by prominently displaying
idom.run()as the first steps to using IDOM shows we are encouraging people to do just that. I believe the primary way of utilizing IDOM should be embedding within existing applications.Proposed Actions
idom.run()as is, but add logging to indicateidom.run()is not suited for production use.idom.run() is only intended for testing purposes. For production use, consider embedding IDOM within an existing project. Check out the docs for more info.PerClientStatetoIdomViewOptional
idom.testing.webserver()idom.shortcuts.run()idom.devtools.run_webserver()idom.run()should raise a simple to understand exception if the user has forgotten to install a web frameworkLooks like you don't have a web framework installed. You'll need to pip install one of the following: fastapi, flask, sanic