Wow this is nice! I did not see this coming... LZ
On Sun, May 7, 2017 at 3:58 PM, Shimon Shtein <ssht...@redhat.com> wrote: > > OK, it wasn't exactly my intention, but I see the discussion moves towards > general plugin install/uninstall procedure. > > I have discovered a nice feature that enables us to isolate plugin > migrations, and run them on per-plugin manner: > Apparently we can run: > rake db:migrate SCOPE=my_plugin > rake db:migrate SCOPE=my_plugin VARSION=0 # to migrate down > And it will run only migrations that are tagged for that scope. One can > achieve that by generating proper migration names: > <timestamp>_<migration_name>.<scope/plugin_name>.rb > I have added a PR that generates migrations for plugins with the proper > naming: https://github.com/theforeman/foreman/pull/4509. > > This should be the base for complete removal. Then, if we add a down-only > migration that removes STI's - it should be enough to remove all DB traces > for a plugin. > > > > On Wed, May 3, 2017 at 2:11 PM, Lukas Zapletal <l...@redhat.com> wrote: >> >> I agree with Tomas, it's more cleaner to remove all the data right >> away. Therefore I suggest that we check for these kind of objects >> during initialization and if such an class is (not) found, then we can >> throw an error like "Run foreman-rake plugin:clean" to delete orhpaned >> records. >> >> I am for (A) - remove all the data. >> >> LZ >> >> On Wed, May 3, 2017 at 11:16 AM, Tomas Strachota <tstra...@redhat.com> >> wrote: >> > This issue is tightly coupled with plugin uninstallation and I think >> > we should solve the two problems together. At the moment there's no >> > standard plugin uninstallation procedure that would take care of >> > cleaning up the system. This is something each plugin should provide. >> > One thing (imho the less important in this context) is where we put >> > such code. Should it be a rake task, part of installer, part of >> > maintanance script, somewhere else? >> > >> > What I see as more important is to decide what data will the >> > uninstallation process remove (keep all vs. keep only settings vs. >> > wipe all) and how we want the plugin to behave if it's installed >> > again. This will affect how we approach missing STI classes. >> > >> > I can see 3 options: >> > >> > a) Remove all data >> > Plugin would remove all tables and records it created. In such case we >> > probably don't need to change the STI behavior. Plugin uninstallation >> > should take care of removing the data correctly. If it fails it's fine >> > to throw exception to indicate the system integrity is broken. This >> > approach is imho safer for us as developers and requires less >> > defensive coding. >> > >> > b) Keep all the data >> > In this case the original data should be present when the plugin is >> > installed again. STI records without classes should be completely >> > hidden in the default scope. If all records are listed we should >> > return either instances of base class or some special class for the >> > missing types. I'm afraid this approach is quite fragile and can lead >> > to many surprises when a plugin is uninstalled, the foreman lives >> > without it for some time and then you install it again. I'm also not >> > sure how common is such usecase. >> > >> > c) Combination of a) and b) >> > We can keep data where it's safe (like settings) and delete the rest. >> > >> > I'm in favor of a) or c) >> > >> > T. >> > >> > >> > >> > >> > >> > On Sun, Apr 30, 2017 at 10:05 AM, <ssht...@redhat.com> wrote: >> >> >> >> Hello, >> >> >> >> lately I was switching plugins on and off, and I stumbled upon an >> >> annoying >> >> issue: Many plugins that add some STI classes (for example Katello that >> >> adds >> >> settings, or Discovery that adds DiscoveredHost). >> >> A problem starts when such a plugin is removed from the system, since >> >> default STI implementation will throw an error if it can't load a class >> >> that >> >> corresponds to the :type column in the DB. >> >> >> >> I propose a custom handling for such cases, since they are not exactly >> >> exceptions in our system design. >> >> I was thinking about one of the following options to handle this case, >> >> and >> >> would like to see a voting which one you prefer. Of course other >> >> options can >> >> be proposed too. >> >> >> >> 1. Return a base class for such records (Maybe with an error already >> >> set) >> >> 2. Return nil/some kind of special AR object for such records (will >> >> require >> >> defensive coding while handling heterogeneous lists) >> >> 3. Filter those records out with default scope (will require more >> >> declaration in plugins, or more DB queries on system startup) >> >> >> >> Any thoughts in this area will be much appreciated, >> >> >> >> Shim >> >> >> >> -- >> >> You received this message because you are subscribed to the Google >> >> Groups >> >> "foreman-dev" group. >> >> To unsubscribe from this group and stop receiving emails from it, send >> >> an >> >> email to foreman-dev+unsubscr...@googlegroups.com. >> >> For more options, visit https://groups.google.com/d/optout. >> > >> > -- >> > You received this message because you are subscribed to the Google >> > Groups "foreman-dev" group. >> > To unsubscribe from this group and stop receiving emails from it, send >> > an email to foreman-dev+unsubscr...@googlegroups.com. >> > For more options, visit https://groups.google.com/d/optout. >> >> >> >> -- >> Later, >> Lukas @lzap Zapletal >> >> -- >> You received this message because you are subscribed to a topic in the >> Google Groups "foreman-dev" group. >> To unsubscribe from this topic, visit >> https://groups.google.com/d/topic/foreman-dev/51oBYLZpw80/unsubscribe. >> To unsubscribe from this group and all its topics, send an email to >> foreman-dev+unsubscr...@googlegroups.com. >> For more options, visit https://groups.google.com/d/optout. > > > -- > You received this message because you are subscribed to the Google Groups > "foreman-dev" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to foreman-dev+unsubscr...@googlegroups.com. > For more options, visit https://groups.google.com/d/optout. -- Later, Lukas @lzap Zapletal -- You received this message because you are subscribed to the Google Groups "foreman-dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to foreman-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
