Cannot instantiate a subcommand if it inherits the same baseclass with a parent-cmd

[ankostis]

Problem

As currently implemented (till 5.x), subapps are actually constructed in ìnitialize_subcommands() by invoking SingletonConfigurable.instance(). And that makes sense, to make it easily to locate a command instance, regardless of its position in the "command chain".

But since this SingletonConfigurable.instance() class-method store the new instance to all baseclasses, this leads easily to clashes in case 2 subcommands share the same baseclass (which is always the case, since Application is a common parent).

Actually from within IPython you cannot even use any sub-command at all:

>>> class App(Application):
...     pass
...
>>> class Sub(Application):
...     pass
...
>>> app = App(subcommands={'sub': (Sub, 'cmd title')})
>>> sub.initialize(['sub'])
Traceback (most recent call last):
...
MultipleInstanceError: An incompatible sibling of 'Sub' is already instanciated as singleton: TerminalIPythonApp

To signify the problem, here is a trivial example that fails even in pure python REPL:

>>> class Sub1(Base):
...   pass
...
>>> class Sub2(Base):
...   pass
...
>>> app = Application(subcommands={'sub1':(Sub1, 'help'), 'sub2': (Sub2, 'help')})

>>> app.initialize(['sub1'])
>>> app.subapp
<__main__.Sub1 object at 0x000001C2D5D77C50>

>>> app.initialize(['sub2'])
Traceback (most recent call last):
...
traitlets.config.configurable.MultipleInstanceError: An incompatible sibling of 'Sub2' is already instanciated as singleton: Sub1

Anothe side-effect is that test-cases on sub-commands require spurious clear_instances() invocations to work (see test_application.test_subcommands_instanciation()).

Possible Mitigations

Either:

1. Modify initialize_subcommands() not to use Singleton.instance() (or use a new method instead?), or
2. modify Singleton.instance() and clear_instances() not to set/clear newly instanciated app to all mro() classe-attributes.

I would prefer (2), assuming there is not some blocker reason for why instance() behaves like that.

Of course it might be that my understanding is totally broken here.

[minrk]

This seems like the right behavior. You have instantiated two singleton instances in the same processes without clearing the singleton. That rightly results in an error because the application singleton must be cleared before you attempt to instantiate another one.

The clear_instances in the tests are not spurious. Since the subcommand creates the instance, it must be cleared before another instance can be created. This is the correct, intended behavior.

[ankostis]

The behavior of the new_instance () correct. I agree.

It's the sub-command behavior that does not make sense. Clearing singleton before launching each command, cancels the whole purpose of a singletons.

[minrk]

That's untrue. If subcommand didn't work, it wouldn't be how most Jupyter and IPython commands are implemented today. A subcommand is an alternate application that should be instantiated as the singleton, e.g. jupyter notebook or ipython kernel or git diff. There shouldn't ever be more than one subcommand instantiated in a single process (same as Application), but if you want to do this (e.g. for testing), then clearing the instance makes sense. If a subcommand isn't the global instance, that would defeat the whole purpose of subcommands.

[ankostis]

I see your point:
Jupyter/IPython always launches a single sub-command on each run
[edit] and code accessing the Application singletons should still receive the sub-command instance.
Is my understanding correct?

But i'm not launching 2 sub-commands either.
It's a single subcommand launching that fails because the baseclass is also it's parent,
so it has already occupied the singleton slot.

There has to be a way out of this, than incidentally forbidding parent & sub-commands to inherit a common baseclass.

[ankostis]

I realized that the title of this issue might have been a little misleading.

[minrk]

There has to be a way out of this, than incidentally forbidding parent & sub-commands to inherit a common baseclass.

There is. We don't forbid subcommands from having a common base class, and IPython subcommands generally do share a base class (BaseIPythonApp). Here's a rough simplification of how IPython subcommands are launched:

from traitlets.config import Application
from traitlets import Unicode

class MyBaseApp(Application):
    common_config = Unicode(config=True)

class MySubApp(MyBaseApp):
    def start(self):
        print("subapp!")

class MyMainApp(MyBaseApp):
    subcommands = {
        'subapp': (
            MySubApp,
            "launch subapp",
        ),
    }

    def start(self):
        if self.subapp is not None:
            return self.subapp.start()
        print("main!")

if __name__ == '__main__':
    MyMainApp.launch_instance(['subapp'])

The instance on MyMainApp is explicitly cleared when initializing the subcommand's Application object to avoid this kind of conflict. The subapp takes over the global instance.

[ankostis]

On IPython the above code still raises:

MultipleInstanceError: An incompatible sibling of 'MyMainApp' is already instanciated as singleton: TerminalIPythonApp

[ankostis]

I noticed that the same problem is reported on my OP, related to TerminalIPythonApp,
because i had run it through ipython interpreter.

But if run it through plain python REPL i got this exception:

traitlets.config.configurable.MultipleInstanceError: An incompatible sibling of 'Sub' is already instanciated as singleton: MySubApp

So my initial posting still holds.

[minrk]

Calling initialize multiple times in one application doesn't have defined behavior, and I wouldn't necessarily expect it to succeed without some between-run cleanup (in this case calling .clear_instance() on the subapp is what's needed).

One change that probably does make sense, and should fix both cases: subapp uses .instance unconditionally, when it should probably be used on the condition that the parent is the current global instance. So only if the parent is the instance should it clear and reassign as is done now. This would actually fix both the above cases exactly as they are because global instances are not in use. It wouldn't fix the hypothetical issue of calling .initialize() to launch subcommands multiple times on the registered global instance, but I don't think that should be supported. This is implemented in #487.