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, <[email protected]> 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 [email protected]. > 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 [email protected]. For more options, visit https://groups.google.com/d/optout.
