|
| 1 | +- Feature Name: Simplify Config Customization |
| 2 | +- Start Date: 2021-03-18 |
| 3 | +- RFC PR: [amundsen-io/rfcs#28](https://github.com/amundsen-io/rfcs/pull/28) |
| 4 | +- Amundsen Issue: [amundsen-io/amundsen#0000](https://github.com/amundsen-io/amundsen/issues/0000) (leave this empty for now) |
| 5 | + |
| 6 | +# Simplify Config Customization |
| 7 | + |
| 8 | +## Summary |
| 9 | + |
| 10 | +Currently customization of API components require defining your own config files, mounting them correctly in docker/k8s and making it avaiable in Amundsen PATH. |
| 11 | +Looking on config paths possible in more mature products (like Airflow or Confluent Kafka) we should enable defining the properties of config classes through environment variables. |
| 12 | + |
| 13 | +Note: |
| 14 | +- This does not change the current behaviour, and just implements another way to define config variables |
| 15 | +- This RFC does not propose any changes for customization of frontend (custom-config.js) |
| 16 | + |
| 17 | +## Motivation |
| 18 | + |
| 19 | +We should aim for simplicity and removing obstacles when it comes to customizing and deploying Amundsen. Reducing the complexity of customizing and deploying amundsen will |
| 20 | +make the path of onboarding Amundsen much easier. |
| 21 | + |
| 22 | +## Guide-level Explanation (aka Product Details) |
| 23 | + |
| 24 | +1. Users still can define configs the way before (backwards compatibility) |
| 25 | +2. There is a mechanism for collecting amundsen specific configuration from environment variables (overriding configuration is possible without a need to define custom config classes). |
| 26 | + |
| 27 | +For example setting env variables: |
| 28 | + |
| 29 | +```shell script |
| 30 | +export AMUNDSEN__METADATA__PROXY_HOST=localhost:21000 |
| 31 | +``` |
| 32 | + |
| 33 | +Would become equivalent of having to: |
| 34 | + |
| 35 | +1. create file `/tmp/configs/amundsen_custom/custom_config.py`: |
| 36 | +```python |
| 37 | +import os |
| 38 | +from amundsen_metadata.config import LocalConfig |
| 39 | + |
| 40 | +class CustomConfig(LocalConfig): |
| 41 | + PROXY_HOST = 'localhost:21000' |
| 42 | +``` |
| 43 | + |
| 44 | +2. set proper env variables |
| 45 | + |
| 46 | +```bash script |
| 47 | +export PATH=$PATH:/tmp/configs/ |
| 48 | +export METADATA_SVC_CONFIG_CLASS=amundsen_custom.custom_config.CustomConfig |
| 49 | +``` |
| 50 | + |
| 51 | +## UI/UX-level Explanation |
| 52 | + |
| 53 | +N/A |
| 54 | + |
| 55 | +## Reference-level Explanation (aka Technical Details) |
| 56 | + |
| 57 | +1. Add `get_config_class(component: string, base_class: Config) -> Config` method to amundsen common |
| 58 | +2. Use `get_config_class` in each service, for example: |
| 59 | + |
| 60 | +```python |
| 61 | +import importlib |
| 62 | +import os |
| 63 | + |
| 64 | +from amundsen.common.config import get_config_class |
| 65 | + |
| 66 | + |
| 67 | +_config_module_class = \ |
| 68 | + os.getenv('METADATA_SVC_CONFIG_MODULE_CLASS') or _config_module_class |
| 69 | + |
| 70 | +config_module = importlib.import_module(''.join(_config_module_class.split('.')[:-1])) |
| 71 | +config_class = getattr(config_module, _config_module_class.split('.')[-1]) |
| 72 | + |
| 73 | +config_module_class = get_config_class('metadata', config_class) |
| 74 | + |
| 75 | +app.config.from_object(config_module_class()) |
| 76 | +``` |
| 77 | + |
| 78 | +Pseudocode: |
| 79 | + |
| 80 | +##### Amundsen Common |
| 81 | + |
| 82 | +```python |
| 83 | +import os |
| 84 | +from json import loads |
| 85 | + |
| 86 | + |
| 87 | +def get_config_class(component, base_class): |
| 88 | + prefix = f'AMUNDSEN__{component.upper()}__' |
| 89 | + |
| 90 | + types_spec = [(str, lambda x: x), |
| 91 | + (int, lambda x: int(x)), |
| 92 | + (list, lambda x: x.split(',')), |
| 93 | + (dict, lambda x: loads(x))] |
| 94 | + |
| 95 | + class ConfigClass(base_class): |
| 96 | + def __init__(self, *args, **kwargs): |
| 97 | + super(ConfigClass, self).__init__(*args, **kwargs) |
| 98 | + |
| 99 | + for k, v in dict(os.environ).items(): |
| 100 | + if k.startswith(prefix): |
| 101 | + setting_name = k.split(prefix)[1] |
| 102 | + |
| 103 | + try: |
| 104 | + current_setting_value = getattr(self, setting_name) |
| 105 | + setting_type = type(current_setting_value) |
| 106 | + except AttributeError: |
| 107 | + print(f'You are trying to set not existing configuration setting: {k}') |
| 108 | + |
| 109 | + continue |
| 110 | + |
| 111 | + new_setting_value = None |
| 112 | + |
| 113 | + for spec in types_spec: |
| 114 | + _type, _function = spec |
| 115 | + |
| 116 | + if issubclass(setting_type, _type): |
| 117 | + new_setting_value = _function(v) |
| 118 | + |
| 119 | + break |
| 120 | + |
| 121 | + if not new_setting_value: |
| 122 | + print(setting_name, setting_type) |
| 123 | + |
| 124 | + continue |
| 125 | + |
| 126 | + setattr(self, setting_name, new_setting_value) |
| 127 | + |
| 128 | + return ConfigClass |
| 129 | +``` |
| 130 | + |
| 131 | +##### Amundsen Metadata |
| 132 | + |
| 133 | +```python |
| 134 | +import os |
| 135 | +from amundsen_common.config import get_config_class |
| 136 | + |
| 137 | + |
| 138 | +class LocalConfig: |
| 139 | + HOST_URL = 'http://localhost:1234' |
| 140 | + LIST_OF_VARS = [] |
| 141 | + INT_SETTING = 0 |
| 142 | + |
| 143 | + |
| 144 | +os.environ['AMUNDSEN__METADATA__HOST_URL'] = 'http://localhost:9999' |
| 145 | +os.environ['AMUNDSEN__METADATA__LIST_OF_VARS'] = 'a,b,c,d,e' |
| 146 | +os.environ['AMUNDSEN__METADATA__INT_SETTING'] = '2' |
| 147 | +os.environ['AMUNDSEN__METADATA__GIGGLES'] = 'http://localhost:9999' |
| 148 | + |
| 149 | +t = get_config_class('metadata', LocalConfig)() |
| 150 | + |
| 151 | +print(t.__dict__) |
| 152 | + |
| 153 | +# Output: |
| 154 | +# You are trying to set not existing configuration setting: AMUNDSEN__METADATA__GIGGLES |
| 155 | +# {'HOST_URL': 'http://localhost:9999', 'LIST_OF_STH': ['a', 'b', 'c', 'd', 'e'], 'SETTING_INT': 2} |
| 156 | +``` |
| 157 | + |
| 158 | +## Drawbacks |
| 159 | + |
| 160 | +N/A |
| 161 | + |
| 162 | +## Alternatives |
| 163 | + |
| 164 | +- Keep current approach forcing users to provide config files, mounting them and keeping in PATH. |
| 165 | +- Make every config option use os.getenv with default value (in my opinon it's worse since there is no centralization of parsing logic) |
| 166 | + |
| 167 | +## Prior art |
| 168 | + |
| 169 | +N/A |
| 170 | + |
| 171 | +## Unresolved questions |
| 172 | + |
| 173 | +N/A |
| 174 | + |
| 175 | +## Future possibilities |
| 176 | + |
0 commit comments