-
Notifications
You must be signed in to change notification settings - Fork 4
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
Profile start/stop API? #13
Comments
Comment by tgoodlet @vodik @moises-silva @jmesquita ping ping? |
Comment by moises-silva @tgoodlet Yeah from reading the first snippet of code, even before reading the second, it kinda felt odd to have stop()/start() methods in what appeared to be a configuration object. This might be a personal perception only but I see configuration & service management as related but different things. You certainly want to restart services (a profile would be a service for example in this case) when changing configuration, but that's sort of a separate responsibility that I'm not even sure should be in the same class/object (manage in this case seems to be doing both). I'd see something like these clearer.
That way you separate configuration management from service/module/command execution. The 'api' object for every module/component at some point could even be smart enough to auto-generate methods based on FS help (if fs help is good enough for some simple cases). Some methods are standard, like: fs.sofia.api.reload() That should be avail for any module cuz it's just a 'reload mod_sofia' or 'reload mod_xxx' I just called the obj api cuz that's what they use for their python/lua bindings, but could be 'control', 'management' or whatever else. So the general gist of the organization is But what you propose at the end looks also good, just less explicit, you're making the api calls an implicit method of the sofia object and the properties implicitly part of the config. |
Comment by vodik I agree, as I see it, I could, in theory define a profile that's completely detached from sandswitches, and then push it: myprofile = make_me_a_profile()
confmng.sofia.profiles['external'] = myprofile # In theory, at least So what does I would suggest a slightly different API: fs = manage('myfshost.com', keyfile='mykey.rsa')
# Deal with configuration explicitly:
config = fs.config()
# ... do stuff with config
fs.load(config) And that leaves fs.sofia.restart_profile(myprofile.name) Just off-the-cuff ideas. |
Comment by tgoodlet @moises-silva @vodik thanks guys both good ideas. |
Comment by tgoodlet @vodik just to clarify regarding:
Is already supported and shown to work with the test here. I do agree that it becomes unclear why the configuration object defines an API while the original Also you suggested:
I like this. My only quiff (besides that it's outside the scope of this issue) is that it requires creating a copy of the currently mapped config sections (not a big deal) and also overwriting only the appropriate sections when The other thing to note that that shorthand for pushing a quick change with your interface looks like: conf = confmng.config()
conf['sofia']['profiles']['external']['settings']['sip-port'] = '9999'
confmng.load(conf) versus confmng.sofia.config['profiles']['external']['settings']['sip-port'] = '9999'
confgmng.commit() Now I totally get the argument for immutability and the corollary pushing state via inputs, but it's still less efficient both memory and run-time wise. That being said I do like it a lot and think it's worth further discussion. What's your opinion on supporting both? Also what about the idea of a sofia_confg = confmng.sofia.config()
dir_conf = confmng.directory.config()
sofia_conf['profiles']['internal']['settings']['sip-port'] = '9999'
dir_conf['default']['groups']['default']['users']['myuser'] = {
'params': {
'password': pw,
'vm-password': pw,
}
}
confmng.sofia.load(sofia_conf)
confmng.directory.load(dir_conf) It means letting the user deal with smaller more manageable piece-wise config data structures. @moises-silva I'm liking the |
Issue by tgoodlet
Wednesday Oct 12, 2016 at 16:39 GMT
Originally opened as sangoma/sandswitches#9
I was thinking about a more clean way to design the API for components in the config which can be started/stopped.
Currently you can do something like:
But I was thinking maybe it makes more sense to have a standard method on the
ConfigManager
or elsewhere that supports this:Any thoughts @moises-silva @vodik?
The text was updated successfully, but these errors were encountered: