Tracking release/workbench-alpha branch to support the workbench

[davemfish]

I just created the new branch release/workbench-alpha - branched from release/3.9. This PR includes the changes needed to support the python side of the invest-workbench. Those changes are listed in #341 and amount to a small flask app that now lives under natcap.invest.web_api natcap.invest.ui_server (thoughts on this name?) and a new invest serve CLI entry-point to start the flask dev server and run the app.

There is a test here to cover the new invest serve subcommand. It imports the new module natcap.invest.ui_server, but so far there are no tests here to cover the flask app's endpoints. We do have tests for those endpoints in the workbench's test suite. And right now the workbench is the only use-case for the natcap.invest.ui_server module. I'm interested to hear opinions about whether those tests should live here in this repo instead.

Fixes #341

Checklist

• Updated HISTORY.rst - Technically this is a user-facing change, with the new CLI entry-point. But we don't exactly expect anyone to use that except us for the workbench. Thoughts?

[emlys]

Looks good to me! I tried out the new invest serve command. Love how you can exit it with ctrl+C 😀 My only comment is about the output of serve:

$ invest serve
 * Serving Flask app "natcap.invest.ui_server" (lazy loading)
 * Environment: production
   WARNING: This is a development server. Do not use it in a production deployment.
   Use a production WSGI server instead.
 * Debug mode: off
 * Running on http://127.0.0.1:56789/ (Press CTRL+C to quit)

What is the 'Environment' setting and if this is a development server, should we change it to say 'Development'?

[davemfish]

Looks good to me! I tried out the new invest serve command. Love how you can exit it with ctrl+C 😀 My only comment is about the output of serve:

$ invest serve
 * Serving Flask app "natcap.invest.ui_server" (lazy loading)
 * Environment: production
   WARNING: This is a development server. Do not use it in a production deployment.
   Use a production WSGI server instead.
 * Debug mode: off
 * Running on http://127.0.0.1:56789/ (Press CTRL+C to quit)

What is the 'Environment' setting and if this is a development server, should we change it to say 'Development'?

Hey @emlys , thanks for bringing this up. By default, flask starts up in production mode. I haven't read about all the implications of that, but one implication is the subsequent WARNING about using the dev server in production. So the options seem to be:

1. leave as-is on the grounds that it's appropriate to warn the user that ran invest serve
2. use a production WSGI server. If this were a "real" webserver that needed to handle concurrent client requests, we would for sure need to do this. And maybe it's a good idea anyway?
3. leave as-is, but on the workbench side we can launch invest serve after setting an env variable to set "Environment: development" as you suggest. That should suppress the WARNING.

[dcdenu4]

Nice!

[dcdenu4]

Hey @davemfish this looks good to me. I'd be fine with punting the production vs development issue Emily brought up into a separate issue. Off the top of my head I'd say number 1 or 3 would be the way to go.

Also, did you add a comment to History? I know James had mentioned maybe adding something.

[emlys]

I agree it's fine to go in a separate issue

[davemfish]

Okay we have passing builds now! Well, except for Mac.

Since you all last looked at this, Doug & I added the optional subargument to specify a port when starting the flask app. e.g. invest serve --port 54321. And there's a new test for that.

Also brought this branch up-to-date with main.

[dcdenu4]

Thanks for the comment!

[dcdenu4]

Thanks Dave!

[phargogh]

Thanks, Dave!

I agree with the other comments about discussing the use of a webserver in a separate thread as well.

[phargogh]

Looks like I'm the last one to submit an approval and all of the builds are passing (except RTD, which is currently an expected failure), so I'll merge this.