Conversation
Ethtool and pyroute2 are not required to be imported before they are actually needed and this change is required for other than linux platforms to be able to run Controller methods where these packages are not required
enhaut
left a comment
enhaut
left a comment
There was a problem hiding this comment.
Have you tried platform-specific imports e.g. https://stackoverflow.com/a/41097342 ?
I guess it broke static analysis?
I thought about it, but there are no alternatives on darwin kernel that would be just "plug-and-play". I am just trying to make LNST work in minimal sense so that darwin kernel is able to load LRC files mostly, not to make it fully functional as that would require quite a lot of effort for minimal to no gain as LNST is currently only being utilized by Linux specific server setups. Also I don't see this happening on Windows NT kernel as there are just way too many references to underlaying system (being linux based), which would break and had to be rewritten to support multiple platforms. Instead I am leveraging FreeBSD properties of Darwin, which allow us to get it somewhat functional. Finally, we already added somewhat of a lazy loading of imports/dependencies at the time of their need/utilization, instead of being always present and this goes along with that idea. |
|
Tests are passing, SimpleNetwork, L2TP and MPTCP all seem to work after the changes. |
| ethtool = "*" | ||
| pyroute2 = "*" | ||
| ethtool = {version = "*", platform = 'linux'} | ||
| pyroute2 = {version = "*", platform = 'linux'} |
There was a problem hiding this comment.
i think if possible i'd also like this to be split in a way that these only install on the Agent installations... and a pure controller installation doesn't need these.
simply for situations when i want to install a pure controller on linux, this will still want to install these dependencies.
| _mandatory_opts = ["tunnel_id", "session_id", "peer_session_id"] | ||
|
|
||
| def __init__(self, ifmanager, *args, **kwargs): | ||
| from pyroute2.netlink import NetlinkError |
There was a problem hiding this comment.
i think we should have a generic Device._import_agent_dependencies method which is then extended in the derived class here to separate it a bit logically from normal instance initialization.
|
|
||
|
|
||
| def scan_netdevs(): | ||
| from pyroute2 import IPRoute |
There was a problem hiding this comment.
i grepped this in the repo and we don't actually use this function anywhere... i think we should just remove it completely...
originally i wanted to know if this was in any way used on the controller, because this seems like an agent only function anyway.
3 participants
Description
Using LNST as dependency when you only require PerfResult structure / importing LRC files it is not required to be installing and using ethtool / pyroute2 dependencies, which are netlink specific - meaning linux specific.
There are multiple LNST users and developers that are not actively using Linux distros and it would simplify working with LNST as dependency.
Tests
8351283
Reviews
@LNST-project/lnst-developers
Closes: #331