-
-
Notifications
You must be signed in to change notification settings - Fork 47
Migrate Webserver to Flask and Jinja templates. #331
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
base: main
Are you sure you want to change the base?
Conversation
|
@brickbots @mrosseel This PR is ready for review and then merge. |
|
Hi @jscheidtmann Thank you for this switch to enable i8n and the great test coverage 🙌 This is a great set of improvements. I have a couple requests and a couple of questions I'll put here and I'll leave a couple of inline notes: Change Requests:
Questions:
|
|
Thanks again @jscheidtmann! Please let me know if you have any questions or thing I'm off base on any of these comments 🤝 |
|
Hi @brickbots - Richard, Re: Indi PoC - yes, that sneaked in. I'll remove it. Re: A/B/C/D vs. LEFT/UP/DOWN/RIGHT and such - I wanted to get the functionality working with the least changes to server.py. I'll change it to the new ones. Re: views / view2 + server/server2 - I developed a parallel implemention, in order to be able to perform a direct comparison, if necessary (primarily to be able to switch back and check, if it's an old bug or newly introduced). I also wanted to be able to run the tests against the old implementation but then decided it's not worth the effort/to spend tokens. I'll change the PR to remove the old implementation. Re: lots of code in ui for serialization - I let claude generate the serialization and it was guided by necessities of the web testing. I did not consider general, dynamic or introspection approaches to serialization. The endpoint's primary purpose is the web testing, and I am not sure how a "good" api for that could look like. E.g. it's rather pythonish at the moment and it would be hard to generate a WSDL for that (if WSDL is still used). If python had conditional compilation, I'd argue that this endpoint should only be defined for development environments. Re: fixtures - Good point. Will do that. I also saw, I did not remove the dependency on botte. :-D |
|
Thanks @jscheidtmann! Yes, I'm also not sure what to do with the UI testing code. It's good to have, and it should not cause much issue at runtime, but it does add to the source file size and probably does impact performance on the main loop at least a little bit. I think we leave it for now to get this all merged and I can circle back when I have some more time to think about it 👍 I've created a ticket so I don't forget: #346 |
|
... from Bottle.
This PR migrates the internal PiFinder webserver to Flask and Jinja and translates the webserver strings.
This PR introduces a /api/current-selection endpoint that provides information on the currently selected menu entry or ui-element. The selenium tests use this to validate the remote control and change the state of the PiFinder.
Known limitations: