#455 Manager CLI doesn't depend on flask-script anymore
Merged by gnaponie. Opened by cqi.
cqi/freshmaker rewrite-cli-argparse  into  master

Download 455.patch

Manager CLI doesn't depend on flask-script anymore

freshmaker/manage.py does not depend on flask-script from now on.
The original freshmaker-* commands are not changed, for example,

freshmaker-upgradedb

Note that, FRESHMAKER_DEVELOPER_ENV have to be set to ensure the dev
config is loaded correctly for development.

During development, those manager commands can also run from manage.py
directly:

python freshmaker/manage.py

Another change happens to command runssl. That is, app.run() is replaced
with the direct call to run_simple to avoid flask logs this warning and
stops the run.

Silently ignoring app.run() because the application is run from the
flask command line executable.

Signed-off-by: Chenxiong Qi cqi@redhat.com

This looks good +1
@mprahl could you review too?

In regards to the FLASK_APP environment variable being needed, I think it's only necessary if you run the flask CLI directly. If you call manage.py or if you keep the console_scripts entries, then the appropriate Flask app will be used.

My preference is to keep those console_scripts entries for that reason and to keep backwards compatibility. We do this in Cachito and it works nicely:
https://github.com/release-engineering/cachito/blob/bcd2a4500910651135bce62954c636a31296fd41/setup.py#L36

rebased onto 18848b75f9530ea6044411db204f69a67e96ea9f

rebased onto ed7bc1ec73b5b9bd002d84bb7bab7728b8126330

rebased onto c34a561895eb4fb19fd4b6419174794adc251331

@mprahl PTAL.

Did you mean to rename this from freshmaker-frontend? I don't see any harm in changing this, but I want to make sure it is intentional.

It looks like the style for imports is stdlib/third-party and then local imports in a separate section. I suggest we keep that here for consistency.

@cqi looks good to me!

rebased onto 928910bf2e9e26e9fd4f56969cf0d8b4a5a61452

@mprahl Both of the comments are addressed.

:thumbsup:

@gnaponie should I go ahead and merge this?

Yes, let's do it.

Pull-Request has been merged by gnaponie

Metadata