CLI-Client for interacting with daemon

Question

I have a daemon which I am able to interact with through a cli-client. The daemon will perform some tasks in future e.g. monitoring the rate at which files are added to a directory.

Right now when I add a new function I need to write

1. The function I would like to trigger
2. A message format which the client can send to the daemon to execute the function
3. A function in the client which takes the users command line argument and turns them in to a string which is sent to the daemon

Is there a better way of designing this interaction?

Below is the daemon

##################### Daemon ##################################
##################### Just for context ########################
from multiprocessing.connection import Listener
class Nark:
 def __init__(self, folder_path):
 self.path = folder_path
 
 def change_path(self, folder_path):
 self.path = folder_path
 
 def get_path(self):
 return self.path
class NarkPool:
 def __init__(self):
 self.narkdict = {}
 
 def add_nark(self, name, path):
 self.narkdict[name] = Nark(path)
 
 def remove_nark(self, name):
 self.narkdict.pop(name)
 
 def change_path(self, name, path):
 self.narkdict[name].change_path(path)
 
 def get_path(self, name):
 return self.narkdict[name].get_path()
 
 def get_names(self):
 return list(self.narkdict.keys())
################### Important Part Below #############################
 
address = ('localhost', 7389)
listener = Listener(address)
narks = NarkPool()
while True:
 conn = listener.accept()
 print ('connection accepted from', listener.last_accepted)
 while True:
 msg = conn.recv()
 command = msg.split(' ') # command input would be eg. add <name> <path>
 # Manually define action to be taken based on each command
 if command[0] == 'add' : narks.add_nark(command[1], command[2])
 if command[0] == 'remove' : narks.remove(command[1])
 if command[0] == 'change_path': narks.change_path(command[1], command[2])
 if command[0] == 'get_path' :
 conn.send(narks.get_path(command[1]))
 if command[0] == 'list':
 conn.send(narks.get_names())
 
 
 if msg :
 conn.close()
 break
listener.close()

And here is the client:

################# Client ##########################
import click
from multiprocessing.connection import Client
adress = ('localhost', 7389)
@click.group()
def cli():
 pass
# for each possible daemon command create a function which
# takes respective user input and sends it to daemon
@cli.command()
@click.argument('name')
@click.argument('path')
def add_nark(name, path):
 conn = Client(adress)
 conn.send(f'add {name} {path}')
 conn.close()
@cli.command()
@click.argument('name')
def remove_nark(name):
 conn = Client(adress)
 conn.send(f'remove {name}')
 conn.close()
@cli.command()
@click.argument('name')
@click.argument('path')
def change_path(name, path):
 conn = Client(adress)
 conn.send(f'change_path {name} {path}')
 conn.close()
@cli.command()
@click.argument('name')
def get_path(name):
 conn = Client(adress)
 conn.send(f'get_path {name}')
 msg = False
 while not msg:
 msg = conn.recv()
 print(msg)
 conn.close()
@cli.command()
def list_runs():
 conn = Client(adress)
 conn.send(f'list')
 msg = False
 while not msg:
 msg = conn.recv()
 print(msg)
 conn.close()
if __name__ == '__main__':
 cli()

Answer 1

Maybe its not the best approach, but it gets the work done:

Intended approach

Instead of having to declare/modify/delete functions every time a command is added/changed/deleted, the idea is to have a list with all command, and then create the corresponding cli commands dynamically.

This way, we could say the commands in your post could be represented as:

from collections import namedtuple
Command = namedtuple(
 'Command',
 (
 'cli_command',
 'serv_command',
 'args',
 'request_responde_and_show_it'
 )
)
COMMANDS = (
 Command(
 cli_command='add_nark',
 serv_command='add',
 args=('name', 'path'),
 request_responde_and_show_it=False
 ),
 Command(
 cli_command='remove_nark',
 serv_command='remove',
 args=('name',),
 request_responde_and_show_it=False
 ),
 Command(
 cli_command='change_path',
 serv_command='change_path',
 args=('name', 'path'),
 request_responde_and_show_it=False
 ),
 Command(
 cli_command='get_path',
 serv_command='get_path',
 args=('name',),
 request_responde_and_show_it=True
 ),
 Command(
 cli_command='list_runs',
 serv_command='list',
 args=(),
 request_responde_and_show_it=True
 ),
)

Now, lets review step by step how we could dynamically declare the cli commands.

Common structure

All of these functions have pretty much the same body, which could be made generic as such:

command = 'add'
args = ('name', 'path')
request_responde_and_show_it = False
with Client(ADDRESS) as conn:
 conn.send(command + ' ' + ' '.join(args))
 if request_responde_and_show_it:
 print(conn.recv())

Dynamically declare functions

Now, knowing this, we can dynamically declare functions for each of our commands.

To dynamically declare functions with a varying number of arguments, I found this approach, but I could not manage to make it work (I know function.func_code has now changed to function.__code__ and function.func_globals to function.__globals__, but still).

So, the approach I found to work is using exec() along with multiline-strings:

function_name = 'add_nark'
args = ('name', 'path')
# Create a string contaning the arguments separated by commas
args_code_str = f'{args}'.replace("'", '').replace('(', '').replace(')', '')
# Declare function on local scope
exec(f'''
def {function_name}({args_code_str}):
 # do stuff here
''')

Annotating functions dynamically

Annotating a function like this:

@my_decorator('some_argument')
def my_function():
 pass

is equivalent to:

def my_function():
 pass
my_function = my_decorator('some_argument')(my_function)

Altogether

from multiprocessing.connection import Client
from collections import namedtuple
import click
Command = namedtuple(
 'Command',
 (
 'cli_command',
 'serv_command',
 'args',
 'request_responde_and_show_it'
 )
)
COMMANDS = (
 Command(
 cli_command='add_nark',
 serv_command='add',
 args=('name', 'path'),
 request_responde_and_show_it=False
 ),
 Command(
 cli_command='remove_nark',
 serv_command='remove',
 args=('name',),
 request_responde_and_show_it=False
 ),
 Command(
 cli_command='change_path',
 serv_command='change_path',
 args=('name', 'path'),
 request_responde_and_show_it=False
 ),
 Command(
 cli_command='get_path',
 serv_command='get_path',
 args=('name',),
 request_responde_and_show_it=True
 ),
 Command(
 cli_command='list_runs',
 serv_command='list',
 args=(),
 request_responde_and_show_it=True
 ),
)
@click.group()
def cli():
 pass
def send_command_to_server(command, args, request_responde_and_show_it):
 print(f'called send_command_to_server with: {command}, {args}, {request_responde_and_show_it}')
 with Client(ADDRESS) as conn:
 conn.send(command + ' ' + ' '.join(args))
 if request_responde_and_show_it:
 print(conn.recv())
def transform_args_to_code_str(args):
 return f'{args}'.replace("'", '').replace('(', '').replace(')', '')
def create_function(command):
 args_code_str = transform_args_to_code_str(command.args)
 # Declare function on local scope
 exec(f'''
def {command.cli_command}({args_code_str}):
 send_command_to_server('{command.serv_command}', ({args_code_str}),
 {command.request_responde_and_show_it})
''')
 created_function = vars()[command.cli_command]
 return created_function
def create_cli_command(command):
 created_function = create_function(command)
 # Equivalent to annotating the created function with @cli.command()
 cli_command = cli.command()(created_function)
 for arg in command.args:
 # Equivalent to annotating cli_command with @click.argument('someargument')
 cli_command = click.argument(arg)(cli_command)
def create_cli_commands(commands):
 for command in commands:
 create_cli_command(command)
if __name__ == '__main__':
 create_cli_commands(COMMANDS)
 cli()

Now, if you wished to, you could even extract the COMMANDS to a json file, so you don't even have to worry about this script when modifying the server

• sev: Thank you so much for this great contribution, the answers I have gotten so far are a lot for me to understand so might take a bit for me to follow up.
• sev: This is already a really big improvement! After thinking about it I had some ideas and would love to hear your feedback: (1) Instead of sending a string I could send a dict with the arguments (2) Maybe I could build the named tuple in the daemon and send it to the client after it first connects (3) If I can send the tuples over, maybe I can instead try to build the whole cli() function in the daemon and send it over to the client
• Miguel Alorda: @sev Question 1 - Yes, that is probably a good idea. Send the whole command as json. That will make it easier to parse in the server, and also easier to read/debug the code. Question 2 - That sounds interesting. That way, you would only declare the messages structure once, and the client wouldn't even need to know what the server commands are. However, I would then stop using cli-command names (add-nark) different from the server-command names (add). As that would mean the server needs to know part of the client's logic.
• Miguel Alorda: @sev Question 3 - I would definetly not build the cli in the server. Then, what is even the point of having a client and a server? The client logic (presentation, in this case), needs to be separated from the server logic (actual processing)
• Miguel Alorda: @sev Actually, now that I think about it, what you are doing is RPC (Remote Procedure Call) through a cli. So, maybe you could actually use xmlrpc (see here), and register the server proxy through fire. That would probably make your client about 10 lines of code!

Answer 2

In your daemon file:

• You are writing getters and setters which are not pythonic because there is no private keyword in python and all attributes are public. Python prefers using @property decorator for meaningful getters and setters, which is one of many kind of descriptors, which might do validation, logging or other tasks before getting or setting the value.

def valid_path(path):
 """Checks if path is valid"""
 pass
class Nark:
 def __init__(self, folder_path):
 # I did not prefix the attribute with an underscore;
 # so the user of the class when accessing gets redirected to the property,
 # and cannot access that attribute without
 # messing with the internal workings of the class like __dict__
 self.path = folder_path
 
 @property
 def path(self):
 return self.path
 
 @path.setter
 def path(self, foldr_path):
 if not valid_path(folder_path):
 raise ValueError("Not a valid path")
 self.path = folder_path

You can even wrap the validation in a decorator, to abstract it.

from functools import wraps
def valid_path(path):
 """Checks if path is valid"""
 pass
def path_must_be_valid(func):
 @wraps(func) 
 def inner(self, path):
 if not valid_path(path):
 raise ValueError("Path must satisfy certain properties")
 
 return func(self, path)
 return inner
class Nark:
 def __init__(self, folder_path):
 # I did not prefix the attribute with an underscore;
 # so the user of the class when accessing gets redirected to the property,
 # and cannot access that attribute without
 # messing with the internal workings of the class like __dict__
 self.path = folder_path
 
 @property
 def path(self):
 return self.path
 
 @path.setter
 @path_must_be_valid
 def path(self, foldr_path):
 self.path = folder_path

• I think creating a NarkPool class is not useful; because the Nark class itself does validation, you can just store the Narks in a dict.

• You are not handling the exceptions that might happen if a key is not in the narkdict, for example: if you try to access a path that doesn't exists it will throw and exception and the daemon will terminate.

You can use the get method on dicts, which accepts a default argument to be returned if the key is not found in the dict.

Or you can use a try except block to handle the exception.

• When using ifs in python all ifs are executed so you are checking the command type 5 times, you can use elif which will stop checking other branches if one of them is executed.

narks = {}
while True:
 conn = listener.accept()
 print ('connection accepted from', listener.last_accepted)
 while True:
 msg = conn.recv()
 # Python has a neat feature called unpacking values from sequences
 try:
 command, path, *additional_params = msg.split(' ') # command input would be eg. add <name> <path>
 except ValueError:
 print("Invalid command")
 try:
 # Manually define action to be taken based on each command
 if command == 'add':
 narks[path] = Nark(additional_parameters[1])
 elif command == 'remove':
 narks.pop(path)
 elif command == 'change_path':
 narks[path].path = additional_parameters[1]
 elif command == 'get_path':
 conn.send(narks[path])
 elif command == 'list':
 conn.send(narks.keys())
 else:
 print(f"I do not recognize the command {command}")
 
 except KeyError:
 print(f"Path {path} is not valid")
 if msg:
 conn.close()
 break
listener.close()

• I would suggest using a tools like pylint, pyflakes or black; to autoformat and lint your code.

In your client file

Your client code is small, so it does not need a lot of changes.

The only thing that I suggest you do, is noticing the similarities between functions and abstract it either using a decorator or a higher order function.

I hope my answer has helped you in any useful way.

Cheers.

• Reinderien: I think that here, the @property offers nothing of value over a plain member variable - for the reasons you've already described. It's just a fancier getter/setter.
• sev: Thank you so much for this great contribution, the answers I have gotten so far are a lot for me to understand to might take a bit for me to follow up. I have now started using pylint and black and also put a github action which uses them to check for code quality now.
• sev: first question: why would I use nark_pool to check the validity of the inputs rather than just checking in nark itself?
• RacketeerHaskeller: @sev After thinking for a while, I think your approach is better, because the Nark class itself should be responsible for validating the paths. I will edit the answer to reflect the changes.