[openwrt] Allow configure DNS resolvers without the need of defining #129
Conversation
…openwisp#117 Fixed the issue by defining two additional classes (DnsServer and DnsSearch) in the interfaces converter. These additional classes are to convert the dns configs when an interface is not provided in the netjson. when an interface is provided, the default converters in the Interface class is used to convert the dns to native config. this was left so because when an interface is defined, other options like DHCP servers might be available and the Interface class handles that very well. Test cases were added to test for these features and also to increase the coverage. Fixes openwisp#117
NoumbissiValere
force-pushed
the
issue/117
branch
from
2e494d6
to
229ace1
Compare
|
@nemesisdesign @atb00ker please can you review this when you have time? Thanks 😄 |
| }) | ||
| expected = self._tabs("""package network | ||
|
|
||
| config dns 'dns' |
There was a problem hiding this comment.
this doesn't look like something that would work on OpenWRT, can you provide the piece of documentation of OpenWRT which explains that this would work? Have you tested this on an actual OpenWRT device?
There was a problem hiding this comment.
Hmmmm, @nemesisdesign . I just followed standards and conversion used in other tests. I don't have an actual OpenWRT device. will love to have one but can't afford it now. Please can you tell me what is wrong with the code so that i try to correct it? thanks
There was a problem hiding this comment.
@NoumbissiValere You can make an OpenWRT VM on your laptop in the meanwhile for testing.
This should help: http://openwisp.io/docs/user/configure-device.html#install-openwrt-on-virtualbox
There was a problem hiding this comment.
Thanks for the help @atb00ker
There was a problem hiding this comment.
The resulting code must be tested on OpenWRT to ensure the desired result is reached, otherwise is not only useless, but harmful, because it adds code that confuses developers and adds configurations on the device that confuse the more experienced OpenWRT users
Ok @nemesisdesign i will get the work together with subsequent work tested on openWRT before sending. Thanks for the corrections |
interfaces #117
Fixed the issue by defining two additional classes (DnsServer and
DnsSearch) in the interfaces
converter. These additional classes are to convert the dns configs when
an interface is not provided in the netjson. when an interface is provided, the default
converters in the Interface class is used to convert the dns to native
config. this was left so because when an interface is defined, other
options like DHCP servers might be available and the Interface class
handles that very well.
Test cases were added to test for these features and also to increase
the coverage.
Fixes #117