Improve plugin loader #1424
Closed
Improve plugin loader #1424
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
As far as I can tell, there was no way previously for an application to register a plugin and ensure that plugin got loaded -- it would be necessary to manually edit the config and specify the plugin be loaded. This is suboptimal; if third party code wishes to track third party extensions on the blockchain, the correct way to do this is with a plugin, and this third party build should be able to load these required plugins regardless of whether the config lists them or not. This commit adds a boolean parameter to application::register_plugin which defaults to false for backwards compatibility; however, if set to true, the plugin will automatically be enabled when the app initializes.
That code was nasty and... kinda wrong. So fix it up all shiny-like. But I also removed the super annoying default "wanted" plugins list, which only causes problems for third parties like me, and in general is just poor form. In my opinion, 5220425 provides a much cleaner way to do this, in a way that is friendly rather than hostile to third parties. Would Be Nice: A generalized plugin conflict system added at the abstract_plugin level, so, for example, the elasticsearch plugin can conflict account_history and we deal with this in a general fashion rather than having this dirty special case check here.
|
Need to send pull requests to I think we will want it, or at least consider it, thank you for sending. I am unsure about the |
|
Replacing... |
Closed
|
thanks, you saved me some work there :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds the ability for client code of the
applicationclass to request plugins be loaded automatically when it registers them. I don't know if upstream bitshares will want this or not, but here's a PR just in case!While this PR is mostly a clean-up/correctness fix (the old conflict detection between the
account_historyandelasticsearchplugins was clumsy and would false-positive if either plugin showed up twice in the list), it does add the rather obvious functionality that thewitness_nodeshould auto-load thewitness_plugineven if the config doesn't explicitly mention it.Please note that this PR does remove the hard-coded list of "wanted" plugins, as this shortcut is annoying to third party code, like that of Follow My Vote, which may not want to be cluttered up by having those plugins linked in. This may cause mild turbulence to existing environments which depend on this shortcut. #sorrynotsorry =) This turbulence can be avoided by calling
register_plugin(true)for the desired auto-load plugins, however, doing so will prevent disabling these plugins from the config.