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

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.

>>> 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.