[Kimchi-devel] [PATCH V2 6/7] add an optional to toggle the sample plugin

[Yu Xin Huo]

On 5/22/2014 2:53 PM, Zhou Zheng Sheng wrote:
> on 2014/05/22 13:44, Yu Xin Huo wrote:
>> On 5/21/2014 11:09 PM, Zhou Zheng Sheng wrote:
>>> on 2014/05/21 18:28, Yu Xin Huo wrote:
>>>> On 5/21/2014 5:19 PM, Zhou Zheng Sheng wrote:
>>>>> on 2014/05/21 14:31, Yu Xin Huo wrote:
>>>>>> On 5/21/2014 11:27 AM, Zhou Zheng Sheng wrote:
>>>>>>> on 2014/05/21 09:22, Sheldon wrote:
>>>>>>>> On 05/20/2014 02:53 PM, Yu Xin Huo wrote:
>>>>>>>>> Sample plugin has no difference from other plugins, it is wrong to
>>>>>>>>> specially
>>>>>>>>> design a command for that sample plugin.
>>>>>>>>> A plugin should have a way to disable itself, I prefer the original
>>>>>>>>> way to
>>>>>>>>> disable a plugin like below.
>>>>>>>>>
>>>>>>>>> In plugin descriptor xml file, comment out all tabs, if no tabs is
>>>>>>>>> defined,
>>>>>>>>> then the plugin will not be loaded.
>>>>>>>>> By this way, no special command is needed, no additional overhead in
>>>>>>>>> coding is
>>>>>>>>> needed.
>>>>>>> @Yu Xin, Your idea of building plugins inside of Kimchi works for the
>>>>>>> plugins that comes with Kimchi originally. However usually third-party
>>>>>>> plugins should be able to build themselves separately and outside from
>>>>>>> kimchi code repository. What Kimchi needs to do is just to discover the
>>>>>>> build result of the pluginX and load it, regardless whether it contains
>>>>>>> tabs or not.
>>>>>>>
>>>>>>> For example, to compile a Linux kernel module, all we need is some
>>>>>>> kernel headers that describe the data structures used by the module
>>>>>>> interface. We do not need the kernel source itself. I have a example
>>>>>>> kimchi plugin that build outside of kimchi in this way. (We are working
>>>>>>> on making it open-source.) After I build the plugin separately, I can
>>>>>>> just copy the build result files to kimchi's plugins dir and it loads
>>>>>>> automatically.
>>>>>>>
>>>>>>> We also do not enforce all plugins to contain tabs, because some
>>>>>>> plugins
>>>>>>> can just extend the kimchi API. And the ".conf" file in the specific
>>>>>>> plugin dir already contains a "enable = True/False" switch. There is no
>>>>>>> need to "comment out all tabs".
>>>>>> Obviously, you mean that there are various types of plugins for kimchi.
>>>>>> If there is a sample for each type of the plugins, no wonder there will
>>>>>> need a command to enable/disable the sample for that type of plugin?
>>>>>>
>>>>> No, we don't need a command to enable the plugins in build time. The
>>>>> third-party plugins are built without Kimchi code, and outside of Kimchi
>>>>> source code directory. You can build Kimchi without any plugins then
>>>>> publish "kimchi.rpm" package. Another developer builds a plugin
>>>>> separately on a different machine in a different time and publish
>>>>> "kimchi-pluginX.rpm". The user download all the RPMs and install.
>>>>>
>>>>> As I said, Kimchi does not provide any plugins, so it does not need to
>>>>> build any plugins, and we don't need any command to enable the plugins
>>>>> in build time. All Kimchi knows is that it scans the plugins directory
>>>>> in runtime and loads all plugins in the directory.
>>>>>
>>>>> In case a third-party developer want to test his plugin without
>>>>> generating and installing a RPM file, he can build the plugin
>>>>> separately, then get Kimchi source code and build Kimchi, copy the
>>>>> plugin build result to Kimchi source code plugins directory, then at
>>>>> last run Kimchi from the source code directory.
>>>>>
>>>>>> If I understand you correctly, you mentioned something like ".conf"
>>>>>> which is configuration file for the plugin to enable/disable itself.
>>>>>> I already stated in my previous mail, I prefer a way to get plugin to
>>>>>> enable/disable itself.
>>>>> You are mixing runtime and build time configurations. The "pluginX.conf"
>>>>> file is for the runtime. After the a plugin is installed, it is enabled
>>>>> by default. If the admin want to disable the plugin temporally, he can
>>>>> edit the "pluginX.conf" file. The build time of the plugins is not
>>>>> related to the build time of Kimchi.
>>>> All the discussion has *nothing* to do with "build time" at all. It is
>>>> all about run-time about how kimchi handle plugin.
>>>> Please *stop* to get anything about "build time" involved to make things
>>>> confusing.
>>> This patch is all about build time configuration. "--enable-plugin-X" is
>>> a switch given to the build scripts. The switch sets the default
>>> availability of a plugin.
>> Already stated that, sample are non-production stuff that should *never* be
>> built into binary delivery package.
>> So no matter whether you enable/disable the sample plugin, it should be left out
>> from binary delivery package.
>>
>> If the command itself is only for build time, the command itself should not be
>> shipped in binary delivery package.
>>
>> For the content of binary delivery, it is determined by the
>> packaging/distribution strategy, it has nothing to do with whether a plugin is
>> enabled or disabled.
>> A by default disabled plugin can also be shipped in final delivery
>>
>> The concept to connect "enable/disable plugin" to "the content of build output"
>> itself does not make any sense.
>> Sample will only be of significance at development time and at run-time.
>>
>>> As regard to runtime configuration, manually
>>> changing "enabled = False/True" is enough. For now the only valid use
>>> case to enable and disable a plugin is that a plugin developer
>>> experimenting some new things on the "sample", and this case is rare. I
>>> think there's little value to add new command for a rare case. Notice
>>> that even the third-party developers do not need to toggle the sample
>>> plugin, and only when we want to extend the plugin framework itself, we
>>> have to enable the sample and test it. So this use case is really rare.
>> Again, for runtime,
>>
>> For development, it is ok to modify some config file because it just follow *my
>> original design* that "*plugin should have a way to disable itself*".
>> For production operations, it is *not* ok for admin to modify the config file, a
>> command is required at runtime to enable/disable plugin which is shipped in
>> binary delivery package.
>>> In all, "--enable-sample" is an acceptable solution for the developer to
>>> toggle the sample plugin occasionally in the developing environment.
>>> It's also acceptable that we don't have any switches or options at all,
>>> and the "enabled = False" is the default in "sample.conf". Whenever the
>>> developer wants to experiment something, just manually edit it and
>>> restart Kimchi back-end server.
>> Stated above,
>>
>> For development, no additional command is needed at all, if I understand you
>> correctly, there is already a way for plugin to disable itself.
>> For the "--enable-sample" command itself, there is no reason to design a command
>> specially for a sample and it need additional handling to leave it out from
>> binary delivery.
>>>> Anything that is not target for production(for example, the sample)
>>>> should *not* be built into the final binary delivery package.
>>> True, the sample plugin didn't, doesn't and won't be in the binary
>>> package. The switch in this patch is just to toggle its availability
>>> when developer runs Kimchi from source code.
>> I am a bit confused here.
>>
>> Here, seems like here you mean that the 'switch' is for run-time.
>>
>> At the beginning, you said below, the 'switch' is for build time.
>>
>> /This patch is all about build time configuration. "--enable-plugin-X" is
>> a switch given to the build scripts. The switch sets the default
>> availability of a plugin./
>>
>> No matter whether you mean run-time or build-time,
>> stated above, sample should *never* be built into binary package, a *switch* is
>> not needed at all.
> Installing a binary package and start Kimchi server is runtime, building
> Kimchi from source code and starting Kimchi server in place without
> installation is also runtime. The latter runtime is use by developers to
> speed up "write -> test -> debug -> write" cycle. The current build
> script doesn't package the sample plugin. The only case you want a
> sample plugin is to build and run Kimchi directly from source code
> directory and experimenting some new staffs against the plugin framework
> itself. This type of activity is rare even when we developing Kimchi and
> plugins. So by default the sample plugin is disabled, and we don't see
> it even when Kimchi is started in place to test our daily development
> changes. This "--enable-sample" option is given to the build scripts
> just to toggle default the availability setting in "sample.conf", and
> "sample.conf" is read by Kimchi in runtime. In other words, this build
> time switch is to toggle the default availability in runtime. Since we
> didn't change the shipping related build scripts, this "--enable-sample"
> only affects the Kimchi server started in place in runtime. In all, it's
> for the "development time", and it's not related to packaging. Having a
> build time switch for this is just for developers convenience.
>
> Considering functionally completeness, it's true that powerful software
> such as Firefox, Jenkins-ci and Linux kernel provide every operation to
> install, load and unload a module. This makes sense only when the
> project core features reach a high completeness, and the core developers
> decides to enable third-party developers to build some add-ons. Look at
> the github issues, Kimchi itself is far from complete. People use for
> its HTML5 accessibility and easy management of the virtualization, not
> for its plugins, and there is no third-party plugin in public at all.
> Frankly speaking, Kimchi for now is far from virt-manager. While there
> is a lot work to do for the core features, the "just works" plugin
> framework is just enough. The requirements for managing plugins are just
> synthetic ones. As I mentioned, the only valid use case for now is just
> experimenting new things on plugin framework itself.
>
> Each task and requirement should be prioritized and the project
> progresses step by step. Each task also has its own border, and it does
> not solve everything. I think for #Issue 342, this 7 patch series has
> already gone too far. If we keep asking completeness, this task would
> never finish. I can accept the current solution in this stage. The
> switch make sense and practical. I can also accept that don't have any
> switch at all because the usage is rare. If you have a lot better ideas
> beyond #Issue 342, you can always start a new mail thread, describe your
> design, write RFC patches, but that does not affect the validity of this
> patch. A fully functional plugin management system can be added later
> after we acknowledge the current progress.
If I read your words correctly, the conclusion got shaped.

1. Never get sample plugin included in binary delivery package.
2. Remove those commands like "enable-sample", "enable-plugins", use 
"sample.conf" to enable/disable plugin.
      Be default, disable sample plugin.

As enable/disable plugin for production operations is not a requirement 
from real client feedback, so I am ok to leave that out currently.

Shaohe, on top of this, please evaluate whether a readme is needed for a 
predictable experience.
>>>> For run-time, 2 scenarios, under development  or staged into production
>>>> for operations.
>>>>
>>>> As you said, the plugin can be installed separately as a binary package
>>>> on top of kimchi installation.
>>>> If the admin want to get rid of any installed plugin, he need a way to
>>>> disable it.
>>> If the user does not want a plugin, he can just uninstalls it.
>> For functionality completeness, a plugin can be
>> installed/uninstalled/enabled/disabled like firebug
>>
>>>> So there is a need to disable plugin. Either 1 or 2 below is ok.
>>>> 1. Design a way for plugin to disable itself.
>>>> 2. Kimchi provide a command to disable/enable any plugin.
>>>>
>>>>>>> @Shaohe, @Yu Xin, as a conclusion, we do not need to add any mechanisms
>>>>>>> to define which plugin to build/load. In Kimchi upstream, all the new
>>>>>>> features should be added to Kimchi itself, not to plugins. The plugins
>>>>>>> are for third-party developers and they should build their plugins
>>>>>>> separately. Only the sample plugin that serves as a example on how to
>>>>>>> write plugins should be treated specially. So --enable-sample is just
>>>>>>> enough, we don't draw the feet of a snake.
>>>>>> Have a command "--enable-sample" specially for a shipped sample in
>>>>>> source distribution is definitely wrong.
>>>>>> There is no justification to have additional overhead in coding for
>>>>>> non-product stuff.
>>>>>>>> Does that mean the user needs to find this file and uncomment these
>>>>>>>> codes when
>>>>>>>> he want to try the plugin.
>>>>>>>>
>>>>>>>> Usually, a user wants to add a new plugin, he can reference the
>>>>>>>> sample plugin codes.
>>>>>>>> He can try it first to how it works and then read the code to how he
>>>>>>>> can make
>>>>>>>> himself plugin.
>>>>>>>>
>>>>>>>> we can change "--enbale-plugins" to "--enable-sample".
>>>>>>>> our switch command("--enable-sample") is also an example, ti tells
>>>>>>>> user how he
>>>>>>>> build a plugin with kimchi
>>>>>>>> together if he just add one plugin.
>>>>>>>>
>>>>>>>> Also he can add "with"  command for his plugins if he wants to add
>>>>>>>> more than one
>>>>>>>> plugin.
>>>>>>>> such as:
>>>>>>>> --with-plugins=plugin1,plugin2,plugin3
>>>>>>>> The  "with"  command is similar to switch command, a plugin developer
>>>>>>>> can read
>>>>>>>> the autotool document to
>>>>>>>> learn more about it.
>