feat: Refactor models as pydantic models #61
Conversation
| self._links.append(i) | ||
| else: | ||
| self._switches.append(i) | ||
| items: conlist(Union[UNI, Link, str], min_items=1) |
There was a problem hiding this comment.
@ajoaoff fyi, there's an initial interesting discussion here #49 (comment) regarding items, let's first also refine and review if other changes will take place just so they're aligned and during code reviews there aren't much back and forth or surprises in terms of requirements, thanks for your initiative to work on this.
cc'ing @Ktmi, @ajoaoff if you could also help out with refining the ideias/blueprint that'd be great.
There was a problem hiding this comment.
@viniarck So I was working to making a model fully compatible with converting from a dict, and I came up with this. This makes it so we can directly convert from our dict/json representation into the Maintanence window object.
However, this will also require updating kytos core to use pydantic models/dataclasses for the id types, as the currently existing ID types aren't fully compatible with pydantic models. The main Issue I encountered was that the string value of the ids were being evaluated before the parameters had been fully parsed, resulting in errors with the LinkID class, and producing the wrong string value for the UNIID class (which I plan to introduce). The string like nature of the current IDs was chosen, because it provides the best backwards compatibility with older code, but it seems like now it may act as a roadblock moving forward.
There was a problem hiding this comment.
@Ktmi, I think maintenance is trying to model more objects and attributes than it should, as you've highlighted initially, probably just the ids potentially as string and a few more attributes should suffice to store what's needed. It's great to try to reuse existing objects, but when they get in the way and would need significant refactoring when we need to reconsider. I don't think that also introducing new models is an issue in the NApps, it also helps to isolate and clearly represents what needs to be stored without having to worry much about that being changed and leak somewhere else (APIs/interface boundaries) where it might be tried to be reused, so partly duplicating some attributes definitions is OK in this case, but, with that said, I think we can simplify based on your initial idea and only store a subset of things that we need like IDs, scheduler attributes and potentially other minor things that need to be discussed/refined (maybe who created the MW?). Let's have a brainstorm session to discuss/align more about it and then you consolidate the options with Antonio as well.
The intention to use pydantic more in the core is also great, maybe gradually in the future we can try to plan for it.
|
Thanks, guys. |
No description provided.