Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feedbacks Wanted] Injection into plugins #1164

Open
dmikurube opened this issue Jul 11, 2019 · 2 comments
Open

[Feedbacks Wanted] Injection into plugins #1164

dmikurube opened this issue Jul 11, 2019 · 2 comments

Comments

@dmikurube
Copy link
Member

@dmikurube dmikurube commented Jul 11, 2019

Currently, we can inject some instances when instantiating plugin instances. But, that Injector is the same with the Embulk's core system. The following instances are injected, for example :

  • org.embulk.config.ModelManager (Do we need this in plugins?)
  • System configs @ForSystemConfig (System configs are system's. Should we use it in plugins?)
  • TempFileSpaceAllocator (We get this through Exec. Not through @Inject.)
  • ...

In reality, very few plugins get injected, for example, embulk-executor-mapreduce which recently got obsolete.


At the same time, plugins are accessing Embulk's features through org.embulk.spi.Exec. It's based on ThreadLocal, and shared through all plugins loaded. In other words, Embulk's core cannot provide different instances for each plugin instance.

For example, we have had the same Logger. We could not configure Loggers for each plugin instance automatically from configs. (If the Logger for each plugin instance had their own tags, it could be useful?)


We think about changing this situation to have two types of Injectors, 1) for Embulk's core system internal, and 2) for plugins. The (2) Injector will have another restricted set of Modules, and in addition, a plugin-specific Exec-like instance to access Embulk's core features.

If your plugin is getting a system instance injected, please let us know. We'll consider to include that instance for the plugin Injectors.

@kamatama41
Copy link
Contributor

@kamatama41 kamatama41 commented Jul 15, 2019

My plugin https://github.com/kamatama41/embulk-executor-remoteserver
(it's a hobby project and no one uses the plugin on production, though 😅 )
uses SystemConfig, ModelManager and ScriptingContainer for the following reason.

The plugin runs Embulk tasks on the remote servers which are different from a server to run Embulk main program. So it needs to copy the contexts of the main program into these servers (content of config.yml, system configs, installed plugins, etc..). I think executor plugins to run Embulk on remote generally have same demand as my plugin and need to refer to the system instances. (Currently, there is no OSS executor plugin other than mapreduce plugin and mine, though. So it might not be important.)

@dmikurube
Copy link
Member Author

@dmikurube dmikurube commented Jul 17, 2019

Thanks for your comment!

Yeah, in general, executor plugins are expected to get affected a lot, unfortunately. The same happened in embulk-executor-mapreduce, and it may be unavoidable in executors. There have been only two OSS executors (mapreduce and remoteserver), though, as you mentioned. ;)

We are giving up the compatibility about executor plugins for a while... sorry about that. After a certain cleanup finishes (v0.10?), some support for executor plugins would come again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants