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

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

Open
woutdenolf opened this issue Sep 4, 2024 · 9 comments
Open

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

woutdenolf opened this issue Sep 4, 2024 · 9 comments

Comments

@woutdenolf
Copy link

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
Copy link
Contributor

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
Copy link
Member

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
Copy link
Member

marcus-oscarsson commented Sep 4, 2024

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
Copy link
Author

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
Copy link
Contributor

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
Copy link
Member

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;

  • gunicorn was not very compatible with CLI arguments,
  • The server that runs with flask-socketio is a production server (when using gevent)
  • Not entirely certain, but I think there was some other gevent related issue as well

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
Copy link
Author

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
Copy link
Contributor

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
Copy link
Member

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 : ) ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants