[FEATURE] multidomain-domainnames #59
Conversation
This reads a given json file containing a dict with bat_device names as keys and domain_names as values.
Given a dcf-path, the provider tries to return the domaincode related to a batadv_dev, like trier did earlier in their code. If it cannot find a match, it returns whatever was provided as 'domaincode' via '-n' in the commandline, which makes it effectively a default value for unkown domains and a nice fallback.
removed unnecessary return variable Co-Authored-By: Martin Weinelt <[email protected]>
|
Ah wait. We lack an entry in the readme file. |
This comment has been minimized.
This comment has been minimized.
|
Hey, thanks for the patch! I'm not sure about using a config file for the batman-device <-> domain association though. While the command line arguments have already gotten far too complex I don't feel comfortable pushing just this part to a conffile. I'd prefer to keep it part of the command line arguments for now. Maybe as |
|
The association itself should be watched carefully, as it goes straight against @genofire s efforts of possibly getting rid of batman and use babel instead, irrc. I do not really agree with the concept of sticking to the pile of commandlinearguments; however if accepting the PR depends on it, I'll implement it for the domain-code. If someone could provide an example of a working commandline call with two? bat-devices and their mesh_ipv4s in the syntax you provided, it would help me a lot.
|
Hey @TobleMiner forget my last request for the commandlinecall, I think I found it in the code. |
|
Well then, this implemented This finally allows the domaincode-file to overwrite the default domain given by Therefore it's: As last thing to allow a start for a proper config-file, I'd like to change the domaincode-file format, so that it's not a dict of domain-codes anymore, but a dict with one key called "domaincodes" and the current dict nested within as its value. |
The file now holds a key called 'domaincodes' and below the assignments. This allows a transition from domaincodefile to configfile in the future, if wanted.
|
@TobleMiner I think, I'm done. Review requested. Currently testing in hanover. Works as intended. |
|
Hi,
Don't get me wrong here, I don't like it either. But I do prefer it over introducing a config file format that is not capable of representing all configuration options of mesh-announce in a proper structure. If we were to introduce the association file right now it would either create a breaking change as soon as a proper config file format is introduced or we would need to keep support for the file around for some time. I'll take it like that but be prepared for me kicking out the association file and replacing it by a proper config for the daemon. I'll test the PR and merge it shortly |
|
LGTM, thanks for implementing! |
This implements the use of domaincode-files, which are simple json dictionaries holding batadv_device-names as keys and the related domain as their value.
If no or an empty json-file is provided, the code falls back to known behavior and returns the value of "-n" which was implemented in #55.
If used this makes the value of '-n' the fallback value.
This PR fixes #56 and provides half of #53.
The idea of binding bat-devices and domaincodes is shamelessly stolen from fftrier:
https://github.com/freifunktrier/mesh-announce/blob/fftr_mods/providers/nodeinfo/system/domain_code.py
The approach of a json was deliberately chosen in order not to spam the commandline-call of respondd.py with further repeating parameters.