hackforla / HomeUniteUs

We're working with community non-profits who have a Host Home or empty bedrooms initiative to develop a workflow management tool to make the process scalable (across all providers), reduce institutional bias, and effectively capture data.
https://homeunite.us/
GNU General Public License v2.0
36 stars 21 forks source link

Add Initial API Configurator #527

Closed ju1es closed 11 months ago

ju1es commented 11 months ago

Problem

We can't specify dynamic configs at runtime depending on the environment.

closes #522

Solution

configs.py

class Config(object):
    HOST = 'localhost'
    PORT = 8080
    DEBUG = True
    USE_RELOADER = True

and personal.py

class PersonalConfig(Config):
    PORT = 8081
    DEBUG = False
    USE_RELOADER = False

then app.config will result in,

{
    'HOST': 'localhost',
    'PORT': 8081,
    'DEBUG': False,
    'USE_RELOADER': False
}

Things to Consider

We can now reference the compiled config via,

from flask import current_app
current_app.config[< config name >]
paulespinosa commented 11 months ago

@ju1es Flask has a Configuration Handling mechanism. Does it meet our needs?

ju1es commented 11 months ago

@paulespinosa this is great, my bad I totally should've considered whatre the options before diving in. i really like the inheritance pattern. i'll implement both and ping ya

ju1es commented 11 months ago

@paulespinosa i'm getting the following,

    app.config.from_object(Config())
    ^^^^^^^^^^
AttributeError: 'FlaskApp' object has no attribute 'config'

i think this is specific to connexion. it'd be nice to use this built in attr. i'll try playing with it a bit more to see if i can get it to work. if it's alright with y'all i don't think we should spend a ton of time on this right now and can re-visit it later if we find an elegant solution.

paulespinosa commented 11 months ago
    app.config.from_object(Config())
    ^^^^^^^^^^
AttributeError: 'FlaskApp' object has no attribute 'config'

i think this is specific to connexion. it'd be nice to use this built in attr. i'll try playing with it a bit more to see if i can get it to work. if it's alright with y'all i don't think we should spend a ton of time on this right now and can re-visit it later if we find an elegant solution.

@ju1es It looks like, according to the connexion source code, we'd have to do something like the following:

def main():
    # `connexion.App` is aliased to `connexion.FlaskApp` (as of connexion v2.13.1)
    # which is the connexion layer built on top of Flask. So we think of it as
    # the connexion app.
    connexion_app = connexion.App(__name__, specification_dir='./_spec/')

    # The underlying instance of Flask is stored in `connexion_app.app`.
    # This is `flask.Flask`.
    # Below, the Flask configuration handler is loaded.
    flask_app = connexion_app.app
    flask_app.config.update(**dotenv_values('.env'))
    flask_app.json_encoder = encoder.JSONEncoder

    # As far as I know at the moment, the configuration needs to be stored in a singleton 
    # such as the Configuration class shown in this PR or similar.
    # I don't know any other way to access the underlying Flask app in the dynamically 
    # loaded modules such as `auth_controller.py`, `admin_controller.py`, etc.
    config = Config()
    config.set_app_config(flask_app.config)

    connexion_app.add_api('openapi.yaml',
                arguments={'title': 'Home Unite Us'},
                pythonic_params=True)
    connexion_app.add_error_handler(AuthError, handle_auth_error)

    # Here we use the configuration stored in the Flask 'layer'.    
    connexion_app.run(port=flask_app.config["port"],
                      debug=flask_app.config["debug"])

In other modules, we might do something like the following:

app_config = Config().get_app_config()

# Initialize Cognito clients
userClient = boto3.client('cognito-idp',
                          region_name = app_config["COGNITO_REGION"],
                          aws_access_key_id = app_config["COGNITO_ACCESS_ID"],
                          aws_secret_access_key = app_config["COGNITO_ACCESS_KEY"])
ju1es commented 11 months ago

OO nice ok yeah we can do this then for loaded modules,

from flask import current_app

current_app.config['CONFIG']
paulespinosa commented 11 months ago

Thank you @ju1es. Can you let us know how "personal" configs are meant to be used, please?

paulespinosa commented 11 months ago

We can't specify dynamic configs at runtime depending on the environment.

From #522

Action Items

  • Implement dynamic configs while apps are running.

Will updating configs during runtime be supported? Thank you.

ju1es commented 11 months ago

The pull request, as it now, appears to support a mix of configuration: configuration in the Config classes and within the .env file because the other modules have not been updated to use current_app.config['CONFIG'] yet. Though, I think you're still working on this PR but just wanted to note it for discussion.

I'd like to update this in other modules in a separate PR. Trying to make smaller PRs so it's easy to digest.

On my first pass at understanding the design, it makes me think the Config classes aren't necessary and can/will introduce secrets into version control. Especially if we do use current_app.config['CONFIG'] in the other modules to get the configs because then the secrets would have to live in the classes that are checked into version control.

this config is not for secrets or any sensitive information for that matter. in the issue I note "Implement dynamic configs for credentials." which would be something like a vault. that's not included in this PR.

Having .env to hold all the configuration is easy for me to understand, makes sense to me when deploying to different environments, and doesn't require code changes. It also is git ignored so nobody should accidentally put secrets into version control.

what happens if we need to set certain configs for prod? how do we maintain version control for prod's .env?

One aspect of this design that is nice is that the configs become attributes that code completion, static analysis, and other tools can help us know what configs are available. Can help us catch typos early. But, unfortunately, it gets lost when we use current_app.config['CONFIG']. This was incorporated in my initial implementation and I do agree with catching typos early so maybe we can use enums here. Might be a little verbose but it's prob worth.

paulespinosa commented 11 months ago

I'd like to update this in other modules in a separate PR. Trying to make smaller PRs so it's easy to digest.

this config is not for secrets or any sensitive information for that matter. in the issue I note "Implement dynamic configs for credentials." which would be something like a vault. that's not included in this PR.

I see, thank you. I wasn't sure because this PR says it closes #522.

Having .env to hold all the configuration is easy for me to understand, makes sense to me when deploying to different environments, and doesn't require code changes. It also is git ignored so nobody should accidentally put secrets into version control.

what happens if we need to set certain configs for prod? how do we maintain version control for prod's .env?

Hmm, do we have a deployment system/procedure in place yet? I think the production version will need to be placed in a secure deployment location that's updated by the person doing deployment. I imagine this will be within an AWS console.

paulespinosa commented 11 months ago

But, unfortunately, it gets lost when we use current_app.config['CONFIG'].

This was incorporated in my initial implementation and I do agree with catching typos early so maybe we can use enums here. Might be a little verbose but it's prob worth.

I'm sorry for not seeing this before suggesting it. I hope a more experienced pythonista can help guide us here.

ju1es commented 11 months ago

But, unfortunately, it gets lost when we use current_app.config['CONFIG'].

This was incorporated in my initial implementation and I do agree with catching typos early so maybe we can use enums here. Might be a little verbose but it's prob worth.

I'm sorry for not seeing this before suggesting it. I hope a more experienced pythonista can help guide us here. @tylerthome?

ju1es commented 11 months ago

@paulespinosa

ju1es commented 11 months ago

@ju1es For most software, the configuration of the software can be modified by external configuration files, environment variables, command line arguments etc. without modifying the software's source code. The software might also hard code default values in the software's source code for the user's benefit.

Will we still be able to modify this software's configuration externally without modifying the source code?

@paulespinosa not with this PR. This would be done in this issue which needs more definition, https://github.com/hackforla/HomeUniteUs/issues/530