✨ feat: add providers for ConfigTrees using pydantic-settings #8
Conversation
guptadev21
requested review from
ankitrgadiya and
smrutisenapati
and removed request for
a team
guptadev21
force-pushed
the
feat/configtree-pydantic
branch
from
e935313 to
62430a5
Compare
guptadev21
force-pushed
the
feat/configtree-pydantic
branch
from
62430a5 to
5b5abd6
Compare
pydantic_source/source.py
Outdated
| self.client = Client(config=config) | ||
| self.tree_name = tree_name | ||
| self.local_file = local_file |
There was a problem hiding this comment.
suggestion: Consider making all the fields and methods private.
| self.client = Client(config=config) | |
| self.tree_name = tree_name | |
| self.local_file = local_file | |
| self._client = Client(config=config) | |
| self._tree_name = tree_name | |
| self._local_file = local_file |
There was a problem hiding this comment.
Some fields and methods are still public.
pydantic_source/source.py
Outdated
| def _extract_data_local(self, input_data: Dict[str, Any] = None) -> Dict[str, Any]: | ||
| for key, value in input_data.items(): | ||
| if isinstance(value, dict): | ||
| if "value" in value: |
There was a problem hiding this comment.
todo: This is incorrect check. This will take the following dictionary:
{
"key": {
"value": 45
}
}
and convert it into this
{
"key": 45
}
It does not check for the presence of metadata. We need to keep the logic exactly like CLI otherwise the behaviour will be incorrect.
There was a problem hiding this comment.
So I will take that method and remove metadata part and return only content.
| config: Configuration, | ||
| tree_name: str = "default", | ||
| key_prefix: str = "", | ||
| api_with_project: bool = True, |
There was a problem hiding this comment.
nitpick: Let's keep it same as the SDK.
| api_with_project: bool = True, | |
| with_project: bool = True, |
| self.config_tree = self.load_config_tree() | ||
|
|
||
| processed_data = self._process_config_tree(raw_data=self.config_tree) | ||
|
|
||
| if processed_data is None: | ||
| raise ValueError("processed_data cannot be None") |
There was a problem hiding this comment.
suggestion: Consider moving this in the _load_config_tree() method itself. We don't use the self.config_tree anywhere except for passing to process_config_tree.
We can call the _process_config_tree inside the load_config_tree and return the processed dictionary directly.
pydantic_source/source.py
Outdated
| def _extract_data_local(self, input_data: Dict[str, Any] = None) -> Dict[str, Any]: | ||
| for key, value in input_data.items(): | ||
| if isinstance(value, dict): | ||
| if "value" in value: |
No description provided.