Skip to content
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

Why is 'name' attribute required? #7

Open
underdpt opened this issue Nov 9, 2016 · 7 comments
Open

Why is 'name' attribute required? #7

underdpt opened this issue Nov 9, 2016 · 7 comments
Labels

Comments

@underdpt
Copy link

underdpt commented Nov 9, 2016

Hi,

I don't know why attribute 'name' is required. It's not mandatory on the configuration file.

Also, please take a look at this pull request from the main repo:

name ended up as a unknown option due to not being part of the if/elif block.

@nMustaki
Copy link
Owner

nMustaki commented Nov 10, 2016

Hi,

are you sure an interface can be unamed ? What's your use case ? I'll check the documentation this weekend.
Currently name is used everywhere to identify interfaces and provides quick access to an adapter; I would hate to remove that ease of use. Besides I don't know if an interface can be unamed in the /etc/network/interfaces file, but I'll check it out this weekend.
In the meantime, can you explain your use case ?

About the PR, I've opened an issue , let's wait a few days to see if its creator wants to join in.

Thanks !

@underdpt
Copy link
Author

Hi,

There's no "use case". It's simply that's the first time I see a "name" parameter on a /etc/network/interfaces setup on Debian. For example, on any server taken randomly:

# The primary network interface
auto eth0
iface eth0 inet static
         address         192.168.0.250
         netmask         255.255.255.0
         broadcast       192.168.0.255
         gateway         192.168.0.254
         up ethtool -s eth0 wol g

If I load this setup with debinterface it works correctly, but automagically creates a 'name' attribute on write, which I think it's harmless but wasn't previously on the interfaces file. I did't take it into account until I hit the bug that was creating 'unknown' attributes again and again.

I think an easy way to make it not mandatory it's to simply check if the attribute was present on loading the configuration file, and simply not save it if not present. Also, removing the mandatory flag on the attribute so the user can remove it before saving. You can maintain the generation of the attribute if it's needed by the rest of the package so nothing gets broken.

If you want to consider this, I can try to send you a PR (but that would be my first PR, so no promises :-) )

@nMustaki
Copy link
Owner

Hum this is a bug, there should not be any "name" parameter written, nor
"unknown" for that's matter. I'll look into it as soon as I can, but if you
want to, go ahead !
Thanks for spotting this

/*
** "What do you despise? By this you are truly known."
** from Manual of Muad'Dib by the Princess Irulan.
*/

On Thu, Nov 10, 2016 at 10:51 AM, underdpt [email protected] wrote:

Hi,

There's no "use case". It's simply that's the first time I see a "name"
parameter on a /etc/network/interfaces setup on Debian. For example, on any
server taken randomly:

The primary network interface

auto eth0
iface eth0 inet static
address 192.168.0.250
netmask 255.255.255.0
broadcast 192.168.0.255
gateway 192.168.0.254
up ethtool -s eth0 wol g

If I load this setup with debinterface it works correctly, but
automagically creates a 'name' attribute on write, which I think it's
harmless but wasn't previously on the interfaces file. I did't take it into
account until I hit the bug that was creating 'unknown' attributes again
and again.

I think an easy way to make it not mandatory it's to simply check if the
attribute was present on loading the configuration file, and simply not
save it if not present. Also, removing the mandatory flag on the attribute
so the user can remove it before saving. You can maintain the generation of
the attribute if it's needed by the rest of the package so nothing gets
broken.

If you want to consider this, I can try to send you a PR (but that would
be my first PR, so no promises :-) )


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#7 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA-fUWgt3eMsPGOYQ17qdkStulHszvoFks5q8ukYgaJpZM4KtdJV
.

@nMustaki nMustaki added the bug label Nov 10, 2016
@nMustaki
Copy link
Owner

Hi,

Also, would you be willing to checkout the devel branch and try again ? If it does not work, could you paste me a complete example, with the code you use and the output ?
I'll look into the "unknown" key to make it better, it's a catchall for all non common debian options but I think I should recursively search inside its content to prevent the case where it has a nested "unknown" key.

Thanks !

@nMustaki
Copy link
Owner

nMustaki commented Dec 7, 2016

@underdpt did you have the time to checkout the dev branch ?

@underdpt
Copy link
Author

I'm sorry i've been out for a few weeks and missed this. I'll try it on next weekend and report here.

@nMustaki
Copy link
Owner

nMustaki commented Dec 12, 2016 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants