Redesign the Plugin Engine

[oeway]

Giving the progress we made on refactoring the plugin engine (thanks to @MartinHjelmare ), I think it's now time to think about organizing the plugin engine in a more flexible and extendable fashion.

For the engine core:

• FileServer: provide file upload/download service, receive command from the engine. Currently there is only http file server available.

• PluginRunner: there are at least 5 types of task runner: 1) subprocess runner which spawn plugin subprocess (the only option we have for now) 2) docker runner which can run plugin with a specified docker image 3) cluster job queue (Slurm/PBS/etc.) 4) Kubernetes runner which calls K8s api to spawn new plugin process from docker image. 5) Worker manager, this is plugin runner because it doesn't spawn new plugin process, but forward the plugin execution config and code to another registered workers (the workers can be another python socket client, or another plugin engine).

• Engine: It consist of a socketio server, forwarding requests to FileServer, PluginRunner, and manage client sessions (browser) and plugin sessions, forward the message between client and plugin sessions .

Note on the worker manager in PluginRunner 5: This aims to solve the network connection issue when a certain plugin engine A is trapped inside a local area network, but still want to be able to access from outside. We can then set plugin engine A in worker mode, and connect to another plugin engine B which is publicly accessible. The user can then run plugins by connecting to engine B but the the job is distributed to plugin A though the worker manager. Therefore to make it work, we need to support: 1) the engine should be able to connect itself as a client to another engine, and register itself as a client (just like a browser client) 2) the engine plugin runner (worker manager ) need to be able to forward plugin execution job to another registered work, and the successive api calls need to be passed to the worker.

For the plugin workers, we need to think about supporting different backends. Not only python, but can be other languages or software tools, one such example is ImageJ.

About data exchange between different plugin processes, currently all the data exchange are supported by two means: 1) most function calls and data are sent through the socketio connection, which is apparently not suitable for large amount of data. 2) The solution for transferring large amount of data for now is to use the FileServer to exchange large amount of data through http upload and download. One thing missing here is a data streaming service described below.

A data streaming service should be provided by the engine, a promising choice would be WebRTC since it support peer-to-peer connection. The p2p is essential because we can avoid data going through the engine all the time, which can be a huge burden for the engine server and also has potential data privacy issue. Api functions can be used to request a channel, and the service should be configured by the engine. Then the engine will be act as a signaling server. The connection involve two peers, it can be one browser client, and one plugin process, two plugin processes, or two browser client. The actual data transmission after is done in the browser client and/or the plugin process. In the browser, WebRTC are natively supported, and in the plugin process aiortc can be used.

[MartinHjelmare]

Great! This overview is really good!

Regarding plugin runners 2-4 do you think they will also use a subprocess to call a program, eg docker, or do you think we'll use python libraries to use those services?

[MartinHjelmare]

Obvious answer to my question is python libraries, I think:

• docker: https://docker-py.readthedocs.io/en/stable/
• slurm: https://github.com/PySlurm/pyslurm: this seem to require some building steps.
• kubernetes: https://github.com/kubernetes-client/python

[MartinHjelmare]

I've sketched up a tree structure based on your overview. I'm not sure about if we should nest the connection and stream under engine.

This is obviously not :100: complete.

(imjoy-engine) :~/Dev/ImJoy-Engine/imjoy$ tree --dirsfirst
.
├── engine
│   ├── connection
│   │   ├── decorator.py
│   │   ├── error.py
│   │   ├── handler.py
│   │   ├── __init__.py
│   │   └── server.py
│   ├── stream
│   │   ├── error.py
│   │   └── __init__.py
│   └── __init__.py
├── file_server
│   ├── error.py
│   ├── http.py
│   └── __init__.py
├── helper
│   └── __init__.py
├── plugin_runner
│   ├── docker.py
│   ├── error.py
│   ├── __init__.py
│   ├── kubernetes.py
│   ├── process.py
│   └── slurm.py
├── util
│   ├── aiohttp.py
│   └── __init__.py
├── const.py
├── error.py
├── __init__.py
├── __main__.py
├── setup.py
└── VERSION

[oeway]

I think the file tree you are sketching is already a good start. Regarding the files inside, my first impression is maybe there will be too many small files and the file tree makes the project looks more complex the it actually does. And I afraid this will pose an extra layer of difficulty when browsing the code base, for example on Github it's not very easy to switch between files when there are many small files wrapped in folders.

I am now wondering if there is good guideline on how to organizing files? A python code base I liked a lot is the Keras (https://github.com/keras-team/keras/tree/master/keras). One thing I appreciate is that the file names has actually meaning (objectives.py, layers/convolutional.py), every thing else are grouped into utils. I find it's very easy to predict file to look when I try to understand something. Therefore, I like the organization of your plugin_runner folder, where you have one file for one runner.

Maybe because I am now get used to the ImJoy single plugin file and also vuejs, I am not sure whether we should extract all the constants and errors and decorators into separate files for each module, especially when we have very few lines for each. For example, I would suggest to start with single file modules, and separate them when it grows to big files or it's semantically meaningful to separate into different files.

This simplified file tree illustrates what I suggest:

imjoy
├── engine.py
├── service
│   ├── file_server.py
│   ├── streaming.py
│   └── __init__.py
├── runner
│   ├── docker.py
│   ├── kubernetes.py
│   ├── subprocess.py
│   ├── slurm.py
│   └── __init__.py
├── util.py
├── __init__.py
├── __main__.py
└── VERSION

This maybe just a personal taste, but I am open to suggestions and wondering if there are actual benefits to adopt a more complex file structure than the above one.

[MartinHjelmare]

I definitely prefer more small modules than few big modules. It's easier to get a grasp of each module and categorize what that module should do. It's easier to do separation of concern. A module that is more than 500 lines is too big for my taste. 100 - 300 lines is good. Of course this will vary depending on content.

We shouldn't put the connection in the engine module. That will be 1000+ lines most likely.

We don't need to start with packages for eg helper and util directly. We can start with modules for those and switch to package when they get too big.

The const module is needed for constants that are used in more than one module or package. Local constants should be kept in the local module or package.

I think we should separate util from helper. Util should not be aware of the project. Helper should be aware. Helpers are functions that aren't a core piece functionality of the other named modules and need to be separated or used in multiple places.

Regarding browsing on github: its a lot easier to browse on mobile with smaller modules. The file filter, t on desktop makes jumping files fast.

[oeway]

What I meant for the problem of browsing comes when we need to switch files and folders, especially when jumping from a file in one subfolder to another subfolder.

I think the reasons for having util and helper separation sounds reasonable, and yes, it would be good to start with file modules.

let's do that then, 300 ~500 sounds like a reasonable threshold to me, and we can make packages later when it grows.

[oeway]

I just noticed that when you say the connection you mean the plugin connection code. No, it should not be part of engine.

Actually it has nothing to do with the engine, it implements an remote procedure call scheme on top of socketio, so I would suggest to call it rpc module.

It is actually a direct port of this javascript file called _JailedSite.js. For example this is the js version of _encode and the python version. For whole ImJoy project, we may want to also keep some consistency between the two versions. For example using the function name, and if we want to use decorators for python, maybe also change the file in js (it seems JS also has decorators, but I never used before ).

I think it's make more sense to separate make a separate pacakge for worker, and put all the stuff which we previously want to call connection as a rpc module and placed next to worker, and we want two versions separated for python2 and 3 so ti will be like this:

imjoy
├── engine.py
├── service
│   ├── file_server.py
│   ├── streaming.py
│   ├── logging.py
│   └── __init__.py
├── runner
│   ├── docker.py
│   ├── kubernetes.py
│   ├── subprocess.py
│   ├── slurm.py
│   └── __init__.py
├── worker
│   ├── rpc.py
│   ├── worker2.py
│   ├── worker3.py
│   └── __init__.py
├── util.py
├── helper.py
├── __init__.py
├── __main__.py
└── VERSION

[MartinHjelmare]

With connection I meant the socketio connection to the browser. Where will that be in your example?

[oeway]

Either in engine.py or we rename it to connection.py. I prefer engine.py because it not only maintain the connection but also manage sessions, corrdinate services and runners.

On Thu 16 May 2019 at 09:13, Martin Hjelmare notifications@github.com wrote:

With connection I meant the socketio connection to the browser. Where will that be in your example?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/oeway/ImJoy/issues/136?email_source=notifications&email_token=AADU3S5FVWIE5N2YIA3I4TTPVUCRPA5CNFSM4HNAHAAKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVQ4PUA#issuecomment-492947408, or mute the thread https://github.com/notifications/unsubscribe-auth/AADU3S3OW22W655VQRAHXBTPVUCRPANCNFSM4HNAHAAA .

[MartinHjelmare]

I think we should not include the connection directly in the engine module. The engine module will be the core of the system, and have more responsibilities than setting up the socketio connection. The connection will also need to take room and be able to grow, eg by adding handlers. We should plan for this.

I think we should have a subpackage under the engine package for the socketio connection, as in my top suggestion. Names can of course change.

[oeway]

My understanding is that the current engine itself is a collection of socketio event handlers, it only do stuff in respond to socketio event. Most of the handlers are part of the engine task, for example register client, launch plugin, start services such as kill process, get file url etc. These are indeed handlers, but what they achieve are the core engine tasks.

Since we use socketio which already simplified the connection management, pure connection code are rather minimal, basically a few lines to setup the server and then the rest are event handlers — which again are tightly coupled with the role of engine.

While we discuss this, maybe we can also try to organize them as you proposed. I guess if you stripe all the event handlers from the engine, there will be nothing left in the engine, but the connection package will become the engine. What’s your thoughts about this?

On Thu 16 May 2019 at 10:12, Martin Hjelmare notifications@github.com wrote:

I think we should not include the connection directly in the engine module. The engine module will be the core of the system, and have more responsibilities than setting up the socketio connection. The connection will also need to take room and be able to grow, eg by adding handlers. We should plan for this.

I think we should have a subpackage under the engine package for the socketio connection, as in my top suggestion. Names can of course change.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/oeway/ImJoy/issues/136?email_source=notifications&email_token=AADU3S473TAQ6HOMSK5JRO3PVUJQBA5CNFSM4HNAHAAKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVRBAXY#issuecomment-492965983, or mute the thread https://github.com/notifications/unsubscribe-auth/AADU3S2KHU5DXWXSR7NP7VTPVUJQBANCNFSM4HNAHAAA .

[MartinHjelmare]

I think there's a need to store the instances of the core features that will be used in different parts of the imjoy package. My suggestion is that we create an Engine instance, a singleton, where we can store the feature instances. We can then pass the Engine instance around have the features available where we need them.

Right now, there is only the config options and the socketio connection, that we need to store. But I think as we grow, there will be more things that we need to store in a central place. Of course we should think carefully about what we store on the Engine instance, and only allow core feature pieces there.

By clearly separating the different parts of the app into different modules it's easier to reason about what happens and what is the responsibility of the different parts. The tree structure of the project will help explain the architecture. By modularizing the code into smaller pieces it's easier to read and understand the code flow. After a module is read, you only need to remember a few interaction points when you go to the next module that connects to the first module.

For a new person, trying to understand the architecture of the program, small pieces separated in a tree structure, makes it easier to learn, in my experience. The new person won't be able to read the whole code in one sitting. It's necessary to start somewhere and move to a couple of different places to get a grasp of the piece of functionality that is interesting at that moment. By limiting the size of the modules, we make it easier for the person to read a whole module, and know that there's nothing in that module that is missed, that could affect the understanding of the piece of functionality that the person is trying to understand.

[oeway]

What you explained sounds good to me, I think it seems you are seeing further than me in terms of how to organize the modules. I suggest let's just take your strategy and move on, when we have issues or better suggestion, we can then adjust.

We can also use this thread to brainstorm more functions and features to be added, we can later design a roadmap for the project.

I just had in mind several other services to add:

1. conda/pip package management
2. virtual environment management
3. docker image management 4 plugin process management
4. workspace management (manage folders etc.)
5. plugin deployment services (related to https://github.com/oeway/ImJoy-Engine/pull/19 and https://github.com/oeway/ImJoy/pull/104 )
6. preview/thumbnail service for files
7. System monitoring to get CPU/GPU usage
8. terminal service (with xterm.js)

There will become modules inside service package and bridged with even handlers.

[oeway]

Just come up with another point that I think it's good to be discussed. I think we should make sure the worker code is self-contained, since there maybe a chance that we will run the workers separately.

For example, when we run a plugin process inside docker, it doesn't make sense to install the entire plugin engine code base, when we only need a small part of the code inside the worker package. I think better design would be support a pip install parameter say pip install imjoy[worker], and only the worker requirements will be installed.

Would be also nice to make the code inside worker self-contained, and the engine won't use modules inside worker and the worker do not use code outside its package.

[MartinHjelmare]

Maybe we should extract the whole worker into a separate project and repo? Then we can publish it as a package on pypi and we do pip install on that package into the worker environment.

[oeway]

Good idea. We need to think a bit more, how we should name the package, and also how should we use the package in the plugin worker.

In a jupyter notebook for example, we would like to develop a plugin and being able to connect to an engine (remotely or locally). We should be able to do this:

from imjoy import api
api.connect('https://xxxxxxx', 'xxxxxtokenxxxxxx')

class ImJoyPlugin:
    ...

api.export(ImJoyPlugin())
api.run_forever()

[oeway]

@MartinHjelmare While you are working on https://github.com/oeway/ImJoy-Engine/pull/61, it makes me think about how we should organize variables and functions in general. Once we have modules such as runners, workers, and services, I think it would make sense to decouple these modules by not depending on global modules (such as const.py and helper.py).

For now we have global modules which hold globally shared variables and functions, one drawback of that is it couples everything use it and any changes to it has a global side-effects.

For variables or functions (e.g. dotdict) which we already used across many modules, I think it make total sense. However, it's less relevant to place module specific variables such as DEFAULT_REQUIREMENTS_PY3 in a separate module. For now, if we change something to the plugin.py and we want to add another requirements, we need to change two files plugin.py and const.py. And we cannot confidently remove stuff in DEFAULT_REQUIREMENTS_PY3 as well, because this variable may be used by other modules as well, or even worse, it can be modified by another module. The same applies to helper functions, we cannot confidently change stuff in global functions before checking the occurrence.

How about let every module manage their own private variable and helper functions, make their name start with _, and only move those already used at least more than twice into those global modules? By doing this, we can reduce the number of files to be changed and also easier to make changes because we know these variables are scoped inside the current module.

[MartinHjelmare]

Yes, that's what we should do. At the moment things aren't all at their proper place while we're refactoring. It will clear up soon.

I don't think we need to name variables and functions with underscore in modules. It is helpful for instance methods though, to know their intended scope.

[oeway]

We already migrated to jupyter server as the plugin engine.