libnetplan: expose the routes list in the netdef

[daniloegea]

Description

I created the NetplanRoute class as a dataclass but I'm not convinced it's a good idea... I feel like it's better to assign each property separately without an __init__ method...

Checklist

• Runs make check successfully.
• Retains 100% code coverage (make check-coverage).
• New/changed keys in YAML format are documented.
• (Optional) Adds example YAML for new feature.
• (Optional) Closes an open bug in Launchpad.

[slyon]

Thanks, lgtm. I left one non-blocking inline comment.

What's your reasoning against the @dataclass, I think it's looking good.

[slyon]

Suggested change

	routes = [route for route in netdef.routes]
	routes = list(netdef.routes)

suggestion (non-blocking): I guess this way it would be easier to read and according to https://www.curs-ml.com/post/fastest-way-to-convert-an-iterator-to-a-list it would even be faster. But OTOH this is a test case, so we can keep it as-is if you want to. The same applies to ip in netdef.routes (and others) above.

[daniloegea]

oh, yeah I think at some point I was calling str() on route and ended up leaving the list comprehension there.

[daniloegea]

So, I think the only thing I didn't like is passing all those objects of the same type to the constructor. One can easily get confused and put them in the wrong place. It's harder to do that using the pattern route.to = to, route.via = via...

But I think the dataclass will be beneficial in other ways, such as comparing the objects for example... let's see I guess

[slyon]

But I think the dataclass will be beneficial in other ways, such as comparing the objects for example... let's see I guess

We could consider calling it class _NetplanRoute for now, so it's easier for us to change the structure in the future. We're our only consumer of that class currently anyway.