2

I have a function which takes the incoming request, parses the data and performs an action and posts the results to a webhook. This is running as background as a Celery Task. This function is a common interface for about a dozen Processors, so can be said to follow the Factory Pattern. Here is the psuedo code:

processors = {
    "action_1": ProcessorClass1, 
    "action_2": ProcessorClass2,
    ...
}

def run_task(action, input_file, *args, **kwargs):
    # Get the input file from a URL
    log = create_logitem()
    try:
        file = get_input_file(input_file)
    except:
        log.status = "Failure"

    # process the input file
    try:
        processor = processors[action](file)
        results = processor.execute()
    except:
        log.status = "Failure"

    # upload the results to another location
    try:
        upload_result_file(results.file)
    except:
        log.status = "Failure"

    # Post the log about the entire process to a webhoook
    post_results_to_webhook(log)

This has been working well for most part as the the inputs were restricted to action and a single argument (input_file). As the software has grown, the processors have increased and the input arguments have started to vary. All the new arguments are passed as keyword arguments and the logic has become more like this.

try:
    input_file = get_input_file(input_file)
    if action == "action_2":
       input_file_2 = get_input_file(kwargs.get("input_file_2"))
except:
    log.status = "failure"


try:
    processor = processors[action](file)
    if action == "action_1":
        extra_argument = kwargs.get("extra_argument")
        results = processor.execute(extra_argument)
    elif action == "action_2":
        extra_1 = kwargs.get("extra_1")
        extra_2 = kwargs.get("extra_2")
        results = processor.execute(input_file_2, extra_1, extra_2)
    else:
        results = processor.execute()
except:
    log.status = "Failure"

Adding the if conditions for a couple of things didn't make a difference, but now almost 6 of the 11 processors have extra inputs specific to them and the code is starting to look complex and I am not sure how to simplify it. Or if at all I should attempt at simplifying it.

Something I have considered:

  1. Create a separate task for the processors with extra inputs - But this would mean, I will be repeating the file fetching, logging, result upload and webhook code in each task.
  2. Moving the file download and argument parsing into the BaseProcessor - This is not possible as the processor is used in other contexts without the file download and webhooks as well.

1 Answer 1

2

Why not treat each argument as legitimate?

input_file = kwargs.get("input_file")
input_file_2 = kwargs.get_or_default("input_file_2")
extra_1 = kwargs.get_or_default("extra1")
extra_2 = kwargs.get_or_default("extra2")

try:
    input_file = get_input_file(input_file)
    if input_file2 != "":
       input_file_2 = get_input_file(input_file_2)
except:
    log.status = "failure"


try:
    processor = processors[action](input_file)
    results = processor.execute(input_file_2, extra_1, extra_2)
except:
    log.status = "Failure"

The whole point of having those processors in an array is because they should be treated identically. Ergo they should all obey the same interface.

Instead of passing in directly, how about a parameters object?

input_file = kwargs.get("input_file")
input_file_2 = kwargs.get_or_default("input_file_2")
extra_1 = kwargs.get_or_default("extra1")
extra_2 = kwargs.get_or_default("extra2")

try:
    input_file = get_input_file(input_file)
    if input_file2 != "":
       input_file_2 = get_input_file(input_file_2)
except:
    log.status = "failure"

params = {
   input_file = input_file
   input_file_2 = input_file_2
   extra_1 = extra_1
   extra_2 = extra_2
}

try:
    results = processors[action].execute(params)
except:
    log.status = "Failure"

This simplifies it even further, as all processors have the same interface now, but each processor only needs to interrogate the arguments they need.

If you want to issue warnings for unused or extraneous params, then its the processors job to report this, as they have the most knowledge about the context.

2
  • Thank you for taking your time to help me out. Your solution simplifies things a lot without having to rewrite the entire thing.
    – Arunmozhi
    Apr 5, 2020 at 11:43
  • 2
    Update: the processor interrogating its requirements actually made me realise something , most of the kwargs I receive are passed unchanged. So I made a copy of the kwargs, update only the ones requiring intermediate processing and pass the copy. Don't have to create a new params with each argument again. Prevents all params going to all processor as well.
    – Arunmozhi
    Apr 5, 2020 at 12:30

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.