-
Notifications
You must be signed in to change notification settings - Fork 43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Do a full type detection when creating cache #527
Conversation
a9f7523
to
9feaedf
Compare
Read used type detection based on ifcfg content before. The user can easily get confused when having e.g. bridge over wlan in such case. Bcs the device is cached as wlan when type is detected according ifcfg but full detection reports that as bridge.
9feaedf
to
6bc62df
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change is short but I don't understand it. @teclator do you?
|
||
- bnc#956755 | ||
- Use full type detection when caching network configuration to | ||
solve ambiguos configurations (like bridge over wlan). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ambiguous
But this is describing implementation details and tells the user nothing about the bug this is fixing or its symptoms.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an internal module which is used mainly by other yast parts. And the change is implementation detail bcs in perfect code internal sorting of NetworkInterfaces' cache would be really internal. Sadly we more or less expose it's organization, so any change in it is public as well.
@@ -789,7 +789,7 @@ def adapt_old_config! | |||
# @param [Hash] a map with netconfig (ifcfg) configuration | |||
# | |||
def add_device(device, ifcfg) | |||
devtype = GetTypeFromIfcfg(ifcfg) || GetType(device) | |||
devtype = GetType(device) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I want to understand the change, I look up the names in http://apitalism.herokuapp.com/?q=GetType
and get
-
http://www.rubydoc.info/github/yast/yast-yast2/Yast%2FNetworkInterfacesClass%3AGetTypeFromIfcfg
Detects device type according given ifcfg configuration -
http://www.rubydoc.info/github/yast/yast-yast2/Yast%2FNetworkInterfacesClass%3AGetType
Detects device type according cached dataIf cached ifcfg for given device is found it is used as parameter for GetTypeFromIfcfgOrName( dev, ifcfg). Otherwise is device handled as unconfigured and result is equal to GetTypeFromIfcfgOrName( dev, nil)
I am still lost. What is the difference in the return value for this particular bug?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Difference is that GetTypeFromIfcfgOrName
uses ifcfg file or name for device type detection. It is just a subset of whole type detection process which is implemented in GetType
. GetType
is the only function which should be present in public API (but it is not bcs of ycp). Particular example of collision is described in the commit message.
The test suite also has something to say, starting here. |
Regarding testsuite. It is failing only in old testsuite, I'll update "out" files. |
@mchf what's the status of this? |
Will be replaced by an updated PR |
It avoids ambiguous categorization of a device like in bsc#956755
another one: https://bugzilla.suse.com/show_bug.cgi?id=1061306