craig-rueda commented on pull request #14109:
URL: https://github.com/apache/superset/pull/14109#issuecomment-820743711


   > Just like Class approach, functional config can also enforce a contract 
with type checking:
   > 
   > ```python
   > from sqlalchemy.types import TypeDecorator
   > 
   > SQLA_ENCRYPTED_FIELD_FACTORY: Callable[[..., Any], TypeDecorator] = ...
   > ```
   > 
   > If using `current_app` in `superset/config.py` is not desired, we can also 
pass `config` as the first argument of the factory function:
   > 
   > ```python
   > SQLA_ENCRYPTED_FIELD_FACTORY: Callable[
   >     [Dict[str, Any], ..., Any],
   >     TypeDecorator
   > ] = lambda config, col, **kwargs: EncryptedType(col, config["SECRET_KEY"], 
**kwargs)
   > ```
   > 
   > ```python
   > from superset import config
   > 
   > def create_encrypted_field(col, **kwargs):
   >     return config["SQLA_ENCRYPTED_FIELD_FACTORY"](config, col, **kwargs)
   > ```
   > 
   > > I'm trying to avoid direct references to app.config inside of Model 
classes via this intermediary
   > 
   > I thought The functional approach also avoids direct access? If putting 
the util function under `models` doesn't feel right, we can easily move it out 
to the global utils. The model class needs this config value to create the 
encrypted field, so there will be a dependency chain one way or another.
   > 
   > As you mentioned, current approach had to jump through several hoops, so I 
feel if would be nicer if we can keep things simple and direct, without 
compromising on the original objectives.
   > 
   > Either way, I think it'd be worth updating `ENCRYPTED_FIELD_TYPE_ADAPTER` 
to `SQLALCHEMY_ENCRYPTED_FIELD_ADAPTER` or `SQLA_ENCRYPTED_FIELD_ADAPTER` so 
it's more obvious this is SQLA related.
   
   This is really just moving the dep somewhere else. You're still importing 
`config` directly, as opposed to delegating to the AppInitializer to handle the 
lifecycle of the app itself. This approach allows use to leverage Dependency 
Injection as opposed to static imports using the Flask App Factory pattern. 
   
   See: https://github.com/apache/superset/issues/8318


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to