ffnord / ffmap-backend

THIS PROJECT DOESN'T HAVE A MAINTAINER!
Other
20 stars 59 forks source link

add commandline option for removal of specific nodes #78

Closed rotanid closed 8 years ago

rotanid commented 8 years ago

this command line option allows you to remove the information about a specific node from the data. the node to be removed is chosen by its MAC address.

tcatm commented 8 years ago

Looks useful! Can you change it to use the node id instead of the mac? The mac may not be present for all nodes.

rotanid commented 8 years ago

actually i forgot to add the "parser.add_argument", fixed it.

as the majority of nodes have a mac, would it be sufficient for you to internally remove the colons from the parameter, if there are any, and use it as the nodeid? if i only have the mac of a node i'd need to manually remove all the colons otherwise before calling the script...

jplitza commented 8 years ago

As usual, I don't see how this is useful. Can you please provide a use case? If possible one where "removing all the colons" is easier than "taking the node id"?

rotanid commented 8 years ago

use case for the whole thing or taking the mac instead of the node id?

if mac: the mac is on the box of the router, it can be used to determine its ipv6 address and thus it's the thing that get's documented by me and others in my community as the nodes individual identifier. the node id on the other hand... i didn't need it so far for anything...

if whole thing: we already have it in our fork, i just thought i contribute upstream - if you don't want it, leave it...

tcatm commented 8 years ago

I guess the common workflow would be to copy&paste the node id from a frontend (e.g. meshviewer). I actually did this manually in the past using jq. There really is no relation between the node id and a node's MAC and nodes aren't required to submit a MAC. Therefore, the node id should be used.

The usecase is valid and the main problem to be solved are race conditions when the backend is run by a cronjob.

rotanid commented 8 years ago

ok, i updated the PR to use the nodeid

can't test it though at the moment

rotanid commented 8 years ago

tested it now and it seems to work like intended

rotanid commented 8 years ago

if you still have objections, please close it instead of ignoring it ;-)