ktmud commented on pull request #14109:
URL: https://github.com/apache/superset/pull/14109#issuecomment-820696527
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 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]
