Python: Add 'factory' support

[gourav-kandoria]

This pr adds the factory support in python languange. By introducing new option as "factory" which can be set as true or false, if true, the callable will be interpreted as factory otherwise not

[gourav-kandoria]

There are the config and module which I used to test config

{
    "applications": {
        "python-app": {
            "type": "python",
            "processes": 2,
            "path": "/www/",
            "targets": {
                "front": {
                    "module": "app",
                    "callable": "app_factory",
                    "factory": true
                },
                "back": {
                    "module": "app",
                    "callable": "app"
                },
                "back_door": {
                    "module": "app",
                    "callable": "app",
                    "factory": false
                }
            }
        }
    },
    "listeners": {
        "127.0.0.1:8100": {
            "pass": "applications/python-app/front"
        },
        "127.0.0.1:8200": {
            "pass": "applications/python-app/back"
        },
        "127.0.0.1:8300": {
            "pass": "applications/python-app/back_door"
        }
    }
}

Python module:

def app(environ, start_response):
    start_response('200 OK', [('Content-Type', 'text/html')])

    return [b'Hell WSGI', b'Hello world']

def application_f(environ, start_response):
    start_response('200 OK', [('Content-Type', 'text/html')])
    return [b'from app created from factory']

def app_factory():
    return application_f

[ac000]

~Please don't open a new pull-request just to make changes, we loose all the previous context!~

~Instead, make changes locally then force push them up. It'll make everyone's life easier...~

OK, I see this is now taking a completely different tact...

[gourav-kandoria]

~Please don't open a new pull-request just to make changes, we loose all the previous context!~

~Instead, make changes locally then force push them up. It'll make everyone's life easier...~

OK, I see this is now taking a completely different tact...

sure will take care of this. actually, the changes were very much different from each other so thought of creating new pr itself

[gourav-kandoria]

@ac000 changes made as suggested

[ac000]

@gourav-kandoria Thanks!

Let me squash those commits down and see what we end up with.. I'll push up the result for you to check...

[ac000]

Hmm, looking at the nxt_conf_vldt_python_members[] array

static nxt_conf_vldt_object_t  nxt_conf_vldt_python_members[] = {               
    {                                                                           
        .name       = nxt_string("module"),                                     
        .type       = NXT_CONF_VLDT_STRING,                                     
        .validator  = nxt_conf_vldt_targets_exclusive,                          
        .u.string   = "module",                                                 
    }, {                                                                        
        .name       = nxt_string("callable"),                                   
        .type       = NXT_CONF_VLDT_STRING,                                     
        .validator  = nxt_conf_vldt_targets_exclusive,                          
        .u.string   = "callable",                                               
    }, {                                                                        
        .name       = nxt_string("factory"),                                    
        .type       = NXT_CONF_VLDT_BOOLEAN                                     
    }, {                                                                        
        .name       = nxt_string("prefix"),                                     
        .type       = NXT_CONF_VLDT_STRING,                                     
        .validator  = nxt_conf_vldt_targets_exclusive,                          
        .u.string   = "prefix",                                                 
    }, {                                                                        
        .name       = nxt_string("targets"),                                    
        .type       = NXT_CONF_VLDT_OBJECT,                                     
        .validator  = nxt_conf_vldt_targets,                                    
        .u.members  = nxt_conf_vldt_python_target_members                       
    },                                                                          

    NXT_CONF_VLDT_NEXT(nxt_conf_vldt_python_common_members)                     
};

It seems none of these options can be used when "targets" is specified, so I will go ahead and add the .validator member to the new factory entry...

I also need to check what the u.string member is used for...

[ac000]

OK, so I'm pretty sure the u.string is what's passed through to the nxt_conf_vldt_targets_exclusive() function as the data argument, so I''ll add that also.

[ac000]

@gourav-kandoria Please check over the updated patch.

Please also check over my attempt at explaining this in the commit message. I think I'm still missing a concrete reason why this is useful.

Commit message and diff of my changes

commit ca04789668ed52234c4c749a209d99c3b306c07f
Author: Gourav <gouravkandoria1500@gmail.com>
Date:   Thu Jun 20 01:10:53 2024 +0530

    python: Add 'factory' support

    This adds support for 'Application Factories' to the Python language
    module.

    This essentially allows you to run some code _before_ the main
    application is loaded. This is similar to the '--factory' setting in
    Uvicorn.

    It can be configured like

      "python-app": {
          "type": "python",
          "path": "/srv/www",
          "module": "create_app",
          "factory": true
      }

    The factory setting defaults to false.

    Closes: https://github.com/nginx/unit/issues/1106
    [ Commit subject, message and some minor code tweaks - Andrew ]
    Signed-off-by: Andrew Clayton <a.clayton@nginx.com>

diff --git ./src/nxt_conf_validation.c ./src/nxt_conf_validation.c
index ed3aa735..96592652 100644
--- ./src/nxt_conf_validation.c
+++ ./src/nxt_conf_validation.c
@@ -840,7 +840,9 @@ static nxt_conf_vldt_object_t  nxt_conf_vldt_python_members[] = {
         .u.string   = "callable",
     }, {
         .name       = nxt_string("factory"),
-        .type       = NXT_CONF_VLDT_BOOLEAN
+        .type       = NXT_CONF_VLDT_BOOLEAN,
+        .validator  = nxt_conf_vldt_targets_exclusive,
+        .u.string   = "factory",
     }, {
         .name       = nxt_string("prefix"),
         .type       = NXT_CONF_VLDT_STRING,
diff --git ./src/python/nxt_python.c ./src/python/nxt_python.c
index 9cb62878..4a08cc64 100644
--- ./src/python/nxt_python.c
+++ ./src/python/nxt_python.c
@@ -403,6 +403,7 @@ nxt_python_set_target(nxt_task_t *task, nxt_python_target_t *target,
     char              *callable, *module_name;
     PyObject          *module, *obj;
     nxt_str_t         str;
+    nxt_bool_t        is_factory = 0;
     nxt_conf_value_t  *value;

     static nxt_str_t  module_str = nxt_string("module");
@@ -451,25 +452,27 @@ nxt_python_set_target(nxt_task_t *task, nxt_python_target_t *target,
     }

     value = nxt_conf_get_object_member(conf, &factory_flag_str, NULL);
-    nxt_bool_t is_factory = 0;
     if (value != NULL) {
         is_factory = nxt_conf_get_boolean(value);
     }
+
     if (is_factory) {
         if (nxt_slow_path(PyCallable_Check(obj) == 0)) {
-            nxt_alert(task, "factory \"%s\" in module \"%s\" can ",
-                            "not be called to fetch callable", 
-                            callable, module_name);
+            nxt_alert(task,
+                      "factory \"%s\" in module \"%s\" ",
+                      "can not be called to fetch callable",
+                      callable, module_name);
             goto fail;
         }

         obj = PyObject_CallObject(obj, NULL);
         if (nxt_slow_path(PyCallable_Check(obj) == 0)) {
-            nxt_alert(task, "factory \"%s\" in module \"%s\" ",
-                            "did not return callable object", 
-                            callable, module_name);
+            nxt_alert(task,
+                      "factory \"%s\" in module \"%s\" ",
+                      "did not return callable object", callable, module_name);
             goto fail;
         }
+
     } else if (nxt_slow_path(PyCallable_Check(obj) == 0)) {
         nxt_alert(task, "\"%s\" in module \"%s\" is not a callable object",
                   callable, module_name);

[callahad]

@ac000 I'm happy to take a crack at explaining it this afternoon. The factory stuff will make sense to Pythonistas.

[callahad]

The Flask docs call out two main use cases:

if you move the creation of [the application object] into a function, you can then create multiple instances of this app later.

So why would you want to do this?

1. Testing. You can have instances of the application with different settings to test every case.
2. Multiple instances. Imagine you want to run different versions of the same application. Of course you could have multiple instances with different configs set up in your webserver, but if you use factories, you can have multiple instances of the same application running in the same application process which can be handy.

So if you've structured your application in that way, for those reasons, it'd be really handy to be able to run it in Unit as well :)

[callahad]

@ac000 here's my crack at a commit message:

python: Support application factories

Adds support for the app factory pattern to the Python language module.
A factory is a callable that returns a WSGI or ASGI application object.

Unit does not support passing arguments to factories.

Setting the `factory` option to `true` instructs Unit to treat the
configured `callable` as a factory.

For example:

    "my-app": {
        "type": "python",
        "path": "/srv/www/",
        "module": "hello",
        "callable": "create_app",
        "factory": true
    }

This is similar to other WSGI / ASGI servers. E.g.,

    $ uvicorn --factory hello:create_app
    $ gunicorn 'hello:create_app()'

The factory setting defaults to false.

Closes: https://github.com/nginx/unit/issues/1106
[ Commit subject, message and some minor code tweaks - Andrew ]
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>

[ac000]

• Use commit message from @callahad

1:  ca047896 ! 1:  cf8620e5 python: Add 'factory' support
    @@ Metadata
     Author: Gourav <gouravkandoria1500@gmail.com>

      ## Commit message ##
    -    python: Add 'factory' support
    +    python: Support application factories

    -    This adds support for 'Application Factories' to the Python language
    -    module.
    +    Adds support for the app factory pattern to the Python language module.
    +    A factory is a callable that returns a WSGI or ASGI application object.

    -    This essentially allows you to run some code _before_ the main
    -    application is loaded. This is similar to the '--factory' setting in
    -    Uvicorn.
    +    Unit does not support passing arguments to factories.

    -    It can be configured like
    +    Setting the `factory` option to `true` instructs Unit to treat the
    +    configured `callable` as a factory.

    -      "python-app": {
    -          "type": "python",
    -          "path": "/srv/www",
    -          "module": "create_app",
    -          "factory": true
    -      }
    +    For example:
    +
    +        "my-app": {
    +            "type": "python",
    +            "path": "/srv/www/",
    +            "module": "hello",
    +            "callable": "create_app",
    +            "factory": true
    +        }
    +
    +    This is similar to other WSGI / ASGI servers. E.g.,
    +
    +        $ uvicorn --factory hello:create_app
    +        $ gunicorn 'hello:create_app()'

         The factory setting defaults to false.

         Closes: https://github.com/nginx/unit/issues/1106
    -    [ Commit subject, message and some minor code tweaks - Andrew ]
    +    [ Commit message - Dan / Minor code tweaks - Andrew ]
         Signed-off-by: Andrew Clayton <a.clayton@nginx.com>

      ## src/nxt_conf_validation.c ##

[ac000]

@andrey-zelenkov

Does this warrant adding a test case for the new Python language module "factory" boolean setting, it defaults to false with no change in behaviour. Setting it to true, well best read the commit message and see the example from above ;)

We'll also need to update the docs...

[ac000]

• Add a link to the example config and app above

$ git range-diff cf8620e5...fcf590cd
1:  cf8620e5 ! 1:  fcf590cd python: Support application factories
    @@ Commit message
         The factory setting defaults to false.

         Closes: https://github.com/nginx/unit/issues/1106
    +    Link: <https://github.com/nginx/unit/pull/1336#issuecomment-2179381605>
         [ Commit message - Dan / Minor code tweaks - Andrew ]
         Signed-off-by: Andrew Clayton <a.clayton@nginx.com>

[callahad]

Definitely needs docs, and I'd strongly prefer at least a simple WSGI test to be included in this patch before we merge.

[gourav-kandoria]

The Flask docs call out two main use cases:

if you move the creation of [the application object] into a function, you can then create multiple instances of this app later. So why would you want to do this?

1. Testing. You can have instances of the application with different settings to test every case.
2. Multiple instances. Imagine you want to run different versions of the same application. Of course you could have multiple instances with different configs set up in your webserver, but if you use factories, you can have multiple instances of the same application running in the same application process which can be handy.

So if you've structured your application in that way, for those reasons, it'd be really handy to be able to run it in Unit as well :)

@callahad, I guess, It should be better, if we are able to give support of arguments to pass to factory, Because, if someone is using factory, they might be using it for the purpose of being able to get any specific callable from the factory. Otherwise, factory without arguments is functionally equivalent to just callable option. These are my opinion. I hope I am not missing anything important here

[ac000]

I was wondering why you couldn't just have your callable call something else...

[gourav-kandoria]

I was wondering why you couldn't just have your callable call something else...

well, the information regarding what to call has to come from somewhere. so factory will return the callable with this info preconfigured. So, I think that's a choice, one may prefer one over the other.

[callahad]

Gunicorn is the only server that supports passing arguments to factories, and even its docs suggest using environment variables instead of args.

I think there's value in merging this as-is, without argument support. That gives us parity with uvicorn, uWSGI, and gunicorn's preferred approach to factories.

[callahad]

gunicorn docs:

Positional and keyword arguments can also be passed, but it is recommended to load configuration from environment variables rather than the command line.

[gourav-kandoria]

Gunicorn is the only server that supports passing arguments to factories, and even its docs suggest using environment variables instead of args.

I think there's value in merging this as-is, without argument support. That gives us parity with uvicorn, uWSGI, and gunicorn's preferred approach to factories.

okay, got your point. So as next step, would add test case of wsgi app as you suggested above. Also do we need to have test for asgi app or not?

[ac000]

okay, got your point. So as next step, would add test case of wsgi app as you suggested above. Also do we need to have test for asgi app or not?

Just one of them should be enough. From what I can tell this is shared code, so if it works for one it should work for both...

[gourav-kandoria]

@ac000 @callahad Could you please help me here. I was trying to run some tests related to python contained in a file. But got error saying "Unit has no python module(s)". Although I have this library "python3.11.unit.so" at the location "./build/lib/unit/modules/".

Am I running the test right way? or do I need to configure something else before running test.

Below is the command and its output.

pytest test/test_python_application.py
=========================================================================== test session starts ===========================================================================
platform linux -- Python 3.11.2, pytest-7.2.1, pluggy-1.0.0+repack -- /usr/bin/python3
cachedir: .pytest_cache
rootdir: /root/projects/unit_45/test, configfile: pytest.ini
collected 41 items                                                                                                                                                        

test/test_python_application.py::test_python_application_variables SKIPPED (Unit has no python module(s))
test/test_python_application.py::test_python_application_query_string SKIPPED (Unit has no python module(s))
test/test_python_application.py::test_python_application_query_string_space SKIPPED (Unit has no python module(s))
test/test_python_application.py::test_python_application_prefix SKIPPED (Unit has no python module(s))
test/test_python_application.py::test_python_application_query_string_empty SKIPPED (Unit has no python module(s))
test/test_python_application.py::test_python_application_query_string_absent SKIPPED (Unit has no python module(s))
test/test_python_application.py::test_python_application_server_port SKIPPED (not yet)
test/test_python_application.py::test_python_application_working_directory_invalid SKIPPED (not yet)
test/test_python_application.py::test_python_application_204_transfer_encoding SKIPPED (Unit has no python module(s))
test/test_python_application.py::test_python_application_ctx_iter_atexit SKIPPED (Unit has no python module(s))
test/test_python_application.py::test_python_keepalive_body SKIPPED (Unit has no python module(s))
test/test_python_application.py::test_python_keepalive_reconfigure SKIPPED (Unit has no python module(s))
test/test_python_application.py::test_python_keepalive_reconfigure_2 SKIPPED (Unit has no python module(s))
test/test_python_application.py::test_python_atexit SKIPPED (Unit has no python module(s))
test/test_python_application.py::test_python_process_switch SKIPPED (Unit has no python module(s))
test/test_python_application.py::test_python_application_start_response_exit SKIPPED (not yet)
test/test_python_application.py::test_python_application_input_iter SKIPPED (Unit has no python module(s))
test/test_python_application.py::test_python_application_input_readline SKIPPED (Unit has no python module(s))
test/test_python_application.py::test_python_application_input_readline_size SKIPPED (Unit has no python module(s))
test/test_python_application.py::test_python_application_input_readlines SKIPPED (Unit has no python module(s))
test/test_python_application.py::test_python_application_input_readlines_huge SKIPPED (Unit has no python module(s))
test/test_python_application.py::test_python_application_input_read_length SKIPPED (Unit has no python module(s))
test/test_python_application.py::test_python_application_errors_write SKIPPED (not yet)
test/test_python_application.py::test_python_application_body_array SKIPPED (Unit has no python module(s))
test/test_python_application.py::test_python_application_body_io SKIPPED (Unit has no python module(s))
test/test_python_application.py::test_python_application_body_io_file SKIPPED (Unit has no python module(s))
test/test_python_application.py::test_python_application_syntax_error SKIPPED (not yet)
test/test_python_application.py::test_python_application_loading_error SKIPPED (Unit has no python module(s))
test/test_python_application.py::test_python_application_close SKIPPED (Unit has no python module(s))
test/test_python_application.py::test_python_application_close_error SKIPPED (Unit has no python module(s))
test/test_python_application.py::test_python_application_not_iterable SKIPPED (Unit has no python module(s))
test/test_python_application.py::test_python_application_write SKIPPED (Unit has no python module(s))
test/test_python_application.py::test_python_application_encoding SKIPPED (Unit has no python module(s))
test/test_python_application.py::test_python_application_unicode SKIPPED (Unit has no python module(s))
test/test_python_application.py::test_python_application_threading SKIPPED (Unit has no python module(s))
test/test_python_application.py::test_python_application_iter_exception SKIPPED (Unit has no python module(s))
test/test_python_application.py::test_python_user_group SKIPPED (Unit has no python module(s))
test/test_python_application.py::test_python_application_callable SKIPPED (Unit has no python module(s))
test/test_python_application.py::test_python_application_path SKIPPED (Unit has no python module(s))
test/test_python_application.py::test_python_application_path_invalid SKIPPED (Unit has no python module(s))
test/test_python_application.py::test_python_application_threads SKIPPED (Unit has no python module(s))

=========================================================================== 41 skipped in 0.24s ===========================================================================

[andrey-zelenkov]

Please make sure that Unit has permission to access modules within this directory. I would start by trying to run unitd with the unitd --modulesdir path/to/build/lib/unit/modules option (this is what tests actually do).

[gourav-kandoria]

Please make sure that Unit has permission to access modules within this directory. I would start by trying to run unitd with the unitd --modulesdir path/to/build/lib/unit/modules option (this is what tests actually do).

I am running the below command to run my build. which I am able to do. Also applying configuration through api also working fine for me.

./build/sbin/unitd

Also tried this way manually it also worked ./build/sbin/unitd --modulesdir "./build/lib/unit/modules"

[ac000]

Did you build the python language module?

To run the python language module pytests it's sufficient to do

$ ./configure
$ ./configure python
$ make
$ pytest test/test_python_*
which: no go in (/home/andrew/.local/bin:/home/andrew/bin:/usr/local/bin:/usr/bin:/usr/local/sbin:/usr/sbin)
which: no go in (/home/andrew/.local/bin:/home/andrew/bin:/usr/local/bin:/usr/bin:/usr/local/sbin:/usr/sbin)
============================= test session starts ==============================
platform linux -- Python 3.12.3, pytest-7.4.3, pluggy-1.3.0 -- /usr/bin/python3
cachedir: .pytest_cache
rootdir: /home/andrew/src/unit/test
configfile: pytest.ini
plugins: anyio-4.2.0, Faker-24.1.0
collected 65 items                                                             
...
======================= 51 passed, 14 skipped in 34.14s ========================

[gourav-kandoria]

Did you build the python language module?

To run the python language module pytests it's sufficient to do

$ ./configure
$ ./configure python
$ make
$ pytest test/test_python_*
which: no go in (/home/andrew/.local/bin:/home/andrew/bin:/usr/local/bin:/usr/bin:/usr/local/sbin:/usr/sbin)
which: no go in (/home/andrew/.local/bin:/home/andrew/bin:/usr/local/bin:/usr/bin:/usr/local/sbin:/usr/sbin)
============================= test session starts ==============================
platform linux -- Python 3.12.3, pytest-7.4.3, pluggy-1.3.0 -- /usr/bin/python3
cachedir: .pytest_cache
rootdir: /home/andrew/src/unit/test
configfile: pytest.ini
plugins: anyio-4.2.0, Faker-24.1.0
collected 65 items                                                             
...
======================= 51 passed, 14 skipped in 34.14s ========================

yes, python language module is built. It was probably due to permission issue. for now got it working by passing the --user root option while running pytest.

[gourav-kandoria]

@ac000 @callahad Pushed test cases related changes also. Could you please review them.

[ac000]

Hi, could you split the test out into a separate patch please?

Is the python targets the right place to put the test?

This isn't specific to targets is it? The added configuration certainly isn't limited to targets. Although the python function modified is nxt_python_set_target() but that may not be specific to targets, need to check...

[ac000]

Right, this is not limited to targets...

[ac000]

This perhaps warrants a new test/test_python_factory.py test... @andrey-zelenkov ? (seeing as the feature we want to test is 'factory'...)

[callahad]

Hi, could you split the test out into a separate patch please?

Curious about why you'd prefer separate commits for the tests?

I do agree these tests should probably be a new thing like test/test_python_factory.py, but the core of the tests look great.

[ac000]

Hi, could you split the test out into a separate patch please?

Curious about why you'd prefer separate commits for the tests?

Adding the functionality is one thing, adding the tests is _another and generally deserve their own commit, it's how I've always done it. The tests also often have a different set of commit tags than the functionality, e.g different reviewers...

[gourav-kandoria]

Hi, could you split the test out into a separate patch please?

Is the python targets the right place to put the test?

This isn't specific to targets is it? The added configuration certainly isn't limited to targets. Although the python function modified is nxt_python_set_target() but that may not be specific to targets, need to check...

sure, will add separate commit for tests. Also if we are planning to add new file like "test/test_python_factory.py". Then I would also add one or more tests handling cases where applying configuration should fail. and ofcourse, will need to add the case, which you highlighted, where "target" is not present

[callahad]

Just a different commit in this same PR is fine :)

[andrey-zelenkov]

This perhaps warrants a new test/test_python_factory.py test... @andrey-zelenkov ? (seeing as the feature we want to test is 'factory'...)

Imo, this feature can be placed in the same row as the callable or prefix options for the Python application, all of which are located in test/test_python_application.py. The reason why test/test_python_target.py has a separate file is that it has its own configuration object. The factory is just a boolean option and can be used without targets, so it can remain in test/test_python_application.py.

[gourav-kandoria]

This perhaps warrants a new test/test_python_factory.py test... @andrey-zelenkov ? (seeing as the feature we want to test is 'factory'...)

Imo, this feature can be placed in the same row as the callable or prefix options for the Python application, all of which are located in test/test_python_application.py. The reason why test/test_python_target.py has a separate file is that it has its own configuration object. The factory is just a boolean option and can be used without targets, so it can remain in test/test_python_application.py.

oh! I just pushed test cases inside test_python_factory.py file. I hope that won't be an issue or should we move these to test_python_target.py file

[ac000]

Hi @gourav-kandoria @andrey-zelenkov (the tests maintainer) is suggesting to put the tests in test/test_python_application.py

Are you OK to do that?

[andrey-zelenkov]

oh! I just pushed test cases inside test_python_factory.py file. I hope that won't be an issue or should we move these to test_python_target.py file

No worries, you can leave it as is (in test_python_factory.py). Just wanted to save you from extra work.

[ac000]

@gourav-kandoria

Looks like there is some error in the new test somewhere, care to take a look?

[gourav-kandoria]

@gourav-kandoria

Looks like there is some error in the new test somewhere, care to take a look?

@ac000 made changes. There was some issue related to assertion failing while reading logs in teardown process. Although tests were passing. so made changes related to that taking reference from other tests

[ac000]

• Revert is_factory change
• Commit message & subject tweaks
• Fix a No newline at end of file issue

$ git range-diff be3d5823...5b06b11f
1:  79216972 ! 1:  047c5eeb python: Support application factories
    @@ src/python/nxt_python.c: nxt_python_set_target(nxt_task_t *task, nxt_python_targ
          char              *callable, *module_name;
          PyObject          *module, *obj;
          nxt_str_t         str;
    -+    nxt_bool_t        is_factory;
    ++    nxt_bool_t        is_factory = 0;
          nxt_conf_value_t  *value;

          static nxt_str_t  module_str = nxt_string("module");
    @@ src/python/nxt_python.c: nxt_python_set_target(nxt_task_t *task, nxt_python_targ
     +    value = nxt_conf_get_object_member(conf, &factory_flag_str, NULL);
     +    if (value != NULL) {
     +        is_factory = nxt_conf_get_boolean(value);
    -+    } else {
    -+        is_factory = 0;
     +    }
     +
     +    if (is_factory) {
2:  be3d5823 ! 2:  5b06b11f Add tests for feature - "Allowing factory supports for python app config"
    @@ Metadata
     Author: Gourav <gouravkandoria1500@gmail.com>

      ## Commit message ##
    -    Add tests for feature - "Allowing factory supports for python app config"
    +    tests: Add tests for python application factories

         Add the following tests cases:
    -    1. When "factory" key is used inside the "targets" option.
    -    2. When "factory" key is used at the root level of python application config.
    -    3. When factory returns invalid callable or When factory is invalid callable

    -    Closes: https://github.com/nginx/unit/issues/1106
    +    1. When "factory" key is used inside the "targets" option.
    +    2. When "factory" key is used at the root level of python application
    +       config.
    +    3. When factory returns invalid callable or When factory is invalid
    +       callable
    +
         Link: <https://github.com/nginx/unit/pull/1336>
    +    [ Commit subject & message formatting tweaks - Andrew ]
         Signed-off-by: Andrew Clayton <a.clayton@nginx.com>

      ## test/python/factory/wsgi.py (new) ##
    @@ test/test_python_factory.py (new)
     +                },
     +            }
     +        )
    - \ No newline at end of file

That is what I'm planning to merge.

This doesn't do

    if (nxt_fast_path(value == NULL)) {                                                                                        

or

    if (nxt_slow_path(is_factory)) {                                                           

As I simply don't see the point here. That is also passing judgement on peoples configuration and will "penalise" (but not really) people who do enable this feature...

[gourav-kandoria]

@ac000 I guess task seems to be completed now. Are there any steps pending, before merge can be done.