mxcube / mxcubeweb

MXCuBE-Web
http://mxcube.github.io/mxcubeweb/
GNU Lesser General Public License v3.0
25 stars 39 forks source link

Canonical flask application or at least a --port CLI argument #1387

Open woutdenolf opened 2 months ago

woutdenolf commented 2 months ago

The canonical way to start a flask application would be this in development

flask --app mxcubeweb.... run [--host 127.0.0.1] [--port 5000] [--debug]

or in production with a proper WSGI server

gunicorn mxcubeweb.... [--host 0.0.0.0] [--port 8000] [--log-level DEBUG]

Would it be possible to support this?

If not, the fast solution for me at the moment would be to add a --port CLI argument.

Side note: the main is located in mxcubeweb/__init__.py instead of the typical mxcubeweb/__main__.py (which would be run when you do python -m mxcubeweb). Any reason for that?

fabcor-maxiv commented 2 months ago

I would welcome a clean up of this. I do not know why it is the way it is now, definitely looked unusual to me when I first started looking at this code. So if possible instead of adding a new CLI option, I would suggest working on making mxcubeweb behave like all other normal Flask apps.

I assume a more canonical structure would also make automated tests easier.

marcus-oscarsson commented 2 months ago

Just for information, be aware that the server started is not the development server but a "production ready" server (https://flask-socketio.readthedocs.io/en/latest/deployment.html#embedded-server)

It would otherwise indeed be very nice if we can run the server in the standard way. I know that we have been discussing it in the past but I can't remember exactly why it was not working or decided to keep it like its currently done.

marcus-oscarsson commented 2 months ago

I had a quick look again and do remember that we did have some issues with passing arguments. Maybe you have some idea on how that should be done as you seem to have some experience with this @woutdenolf and @fabcor-maxiv

woutdenolf commented 2 months ago

some issues with passing arguments

You mean you want to have additional CLI arguments in addition to when the flask or gunicorn CLI provides?

That's indeed a little tricky. I've done this for FastAPI but never for Flask. FastAPI uses click and you can either extend it (which is a little hacky) or use pydantic_settings but then the arguments need to come from environment variables. Never tried for Flask. It seems to use click as well https://flask.palletsprojects.com/en/3.0.x/cli/.

fabcor-maxiv commented 2 months ago

I would suggest environment variables.

MXCUBE_WEB_UI=/path/to/ui/build MXCUBE_HWO_REPO=/path/to/hwos gunicorn mxcubeweb ...

Would that work? There is also the whole dotenv solutions as well, which seems to be an (indirect) dependency already anyway.

marcus-oscarsson commented 2 months ago

My memory is somehow coming back to me after reading some of the issues with gunicorn and CLI arguments. At the time we decided to keep things as they where i.e not starting using gunicorn because;

At least that gives you some historical perspective. Its maybe worth trying to get things to work in a more standardized fashion, it would at least be more elegant.

It seems to me that environment variables is a common solution, as there are multiple processes involved. However, its maybe more a personal opinion, but I find it quite awkward to export a set of variables before starting the application. I would in that case prefer a helper that read the settings from a configuration file that then exports and starts the application. Alternatively that the backend reads the configuration file directly.

There is also some way to create a custom launcher but that would only work for gunicorn I guess.

woutdenolf commented 2 months ago

For reference

Application configuration: https://flask.palletsprojects.com/en/latest/config/

dotenv is only mentioned in relation to the flask CLI: https://flask.palletsprojects.com/en/latest/cli/#environment-variables-from-dotenv

fabcor-maxiv commented 2 months ago

dotenv is only mentioned in relation to the flask CLI: https://flask.palletsprojects.com/en/latest/cli/#environment-variables-from-dotenv

This page also says this:

If you would like to load these files when running in production, you should call load_dotenv() manually.

marcus-oscarsson commented 2 months ago

I'm not promising anything, but I might start looking into this or at-least parts of this relativity soon. I just wanted to make sure that no one else is doing the same. If its the case please let me know : ) ?