search

clicon / clixon

YANG-based toolchain including NETCONF and RESTCONF interfaces and an interactive CLI
http://www.clicon.org/
Other
208 stars 70 forks source link

Add autoconf handling of clixon user/group in config files #442

Closed pprindeville closed 1 year ago

pprindeville commented 1 year ago

Pass the user/group into configure so that the .xml.in file can get AC_SUBST()'d appropriately.

olofhagsand commented 1 year ago

I have some problems with this. The user/group is in the config file and defined in the clixon config YANG: https://github.com/clicon/clixon/blob/master/yang/clixon/clixon-config%402023-05-01.yang : CLICON_SOCK_GROUP / CLICON_BACKEND_USER / CLICON_RESTCONF_USER. The default value of CLICON_SOCK_GROUP is "clicon" and CLICON_RESTCONF_USER is "www-data" We can discuss that, maybe they should not have default values? The autoconf change proposed in this PR affects only individual examples / applications of clixon. In the clixon repo (here), it affect only the main example. It currently exists because some tests use it. I even plan to make the tests independent and then move the main example to clixon-examples. It could make sense to move the PR there, and to other clixon applications, such as clixon-openwrt/clixon-controller, orin the documentation. But not in the core clixon module. And the reason is that it is application-dependent. Having default values in the YANG may even be wrong. Adding an autoconf value on top of the default value is confusing.

pprindeville commented 1 year ago

I have some problems with this. The user/group is in the config file and defined in the clixon config YANG

Huh. I don't understand specifying a default value in the YANG. The YANG defines the protocol aspects, and should be independent of any distro-specific details, of which the associated user/group is but one.

We can discuss that, maybe they should not have default values?

Yeah, I'm inclined to believe that these should be passed in via the distro's packaging. In that case, dropping the default values makes sense... both from configure.ac and from the YANG.

Having default values in the YANG may even be wrong.

Yup, agreed.

Okay, I'll close this PR and let you make the changes you had planned.