Refactor Container Index JSON Loader: Move to Config Package & Switch to Pydantic #1092
Conversation
mrDzurb
requested review from
darenr,
mayoor,
VipulMascarenhas,
qiuosier and
ahosler
as code owners
oracle-contributor-agreement
bot
added
the
OCA Verified
|
📌 Cov diff with main: 📌 Overall coverage: |
|
📌 Cov diff with main: 📌 Overall coverage: |
|
📌 Cov diff with main: 📌 Overall coverage: |
|
📌 Cov diff with main: 📌 Overall coverage: |
|
📌 Cov diff with main: 📌 Overall coverage: |
ads/aqua/extension/model_handler.py
Outdated
| @@ -45,7 +38,7 @@ def get( | |||
| 400, Errors.MISSING_REQUIRED_PARAMETER.format("model_format") | |||
| ) | |||
| try: | |||
| model_format = ModelFormat(model_format.upper()) | |||
| model_format = model_format.upper() | |||
There was a problem hiding this comment.
Seems like this code is no longer checking if the model format is valid? In that case, I guess there is no need to have try/except for upper()?
| ------- | ||
| AquaContainerConfig: The constructed container configuration. | ||
| """ | ||
| # if not config: |
There was a problem hiding this comment.
Do we need this? I see you are adding config=get_container_config() in every call instead. So the config should not be None anymore. Or maybe having a default is better?
|
📌 Cov diff with main: 📌 Overall coverage: |
mrDzurb
dismissed stale reviews from kumar-shivam-ranjan, qiuosier, and mayoor
via
dd15934
|
📌 Cov diff with main: 📌 Overall coverage: |
|
📌 Cov diff with main: No lines with coverage information in this diff. 📌 Overall coverage: |
Description
This PR enhances the container index JSON loader by moving it from the UI module to the config package and refactoring it to use Pydantic instead of dataclasses.
Key Changes