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

PF4J 3 roadmap #282

Closed
2 of 3 tasks
decebals opened this issue Feb 26, 2019 · 16 comments
Closed
2 of 3 tasks

PF4J 3 roadmap #282

decebals opened this issue Feb 26, 2019 · 16 comments

Comments

@decebals
Copy link
Member

decebals commented Feb 26, 2019

I think that is time to talk about PF4J 3.x roadmap.
PF4J is maintained under the Semantic Versioning guidelines, so in a new major version (3.0 in our case) we can make incompatible changes in our API.
Now is time to improve PF4J without to care about backward compatible.

I will try to add some proposes:

The list with proposes will grow in time.

If you think that something is bad designed or you want to change something in the current API, please add a comment.

@dmitry-timofeev
Copy link
Contributor

Hi @decebals , thanks for putting together and leading this project!

Some possible issues to consider from a new user perspective:

  1. Support providing instances of any interface as an ExtensionPoint (Support any interface as an ExtensionPoint #289)
  2. Allow to load plugin with same artifactIds (PluginIds) (or update the docs to recommend other PluginId interpretation) (Allow to load plugin with same artifactIds/PluginIds #290)
  3. Communicate errors with Exceptions where appropriate (Communicate errors with Exceptions where appropriate #292)

@decebals
Copy link
Member Author

decebals commented Mar 13, 2019

I started to work on PF4J 3. The modifications are available on pf4j_3 branch.
The development is done on Java 11 (OpenJDK) but the compiled classes will be compatible with Java 8.

@decebals
Copy link
Member Author

Migrated tests to JUnit 5 with e1fb3f7 commit.
The next step is to solve #292 (Communicate errors with Exceptions where appropriate).

@pinhead84
Copy link
Contributor

Maybe you can create a milestone for PF4J 3 and assign all related issues to it? - Maybe this would make it more transparent, which issues need to be resolved (or are already resolved) for the next major release.

@decebals
Copy link
Member Author

decebals commented Mar 25, 2019

Maybe you can create a milestone for PF4J 3 and assign all related issues to it?

The idea is good but I don't have a fixed list with all features. From this reason I cannot supply a real evolution of this work. For example, I will include #269 only if I receive a PR for this. I don't want to add more and more features only to increase the features list. If more users vote for a feature, then this feature is a serious candidate to be included in the next versions.

@pinhead84
Copy link
Contributor

The idea is good but I don't have a fixed list with all features. From this reason I cannot supply a real evolution of this work.

I don't expect a final / fixed list of features on the milestone. Just assign all issues to the milestone, that you think are plausible / doable for the PF4J 3 release. Currently it's not very transparent,

  • which changes were already made,
  • which changes are planned to be made and
  • which changes need help by others.

Issues like #269, that you can't (or don't want) implement by yourself, can have an additional "help wanted" label. Everybody is invited to tackle those issues. Otherwise if the planned features are implemented, all unresolved "help wanted" issues are removed from the milestone and PF4J 3 is ready for release.

If I find the time, I would like to do some more testing and maybe try to implement one of these issues. But currently I'm quite confused and don't have a complete overview about the state of development.

@decebals decebals changed the title PF4J 3.x roadmap PF4J 3 roadmap Mar 26, 2019
@decebals
Copy link
Member Author

decebals commented Mar 26, 2019

I created PF4J 3 milestone and added some issues (maybe not all).

@decebals
Copy link
Member Author

PF4J 3 is ready to be released (zero open issues for this milestone).
I released a new SNAPSHOT version (3.0.0-SNAPSHOT). Please test a little bit the new version and give me a feedback. Thanks!

@dmitry-timofeev
Copy link
Contributor

Hi @decebals ,

Thanks for the updates! I've successfully upgraded our code and tests to the current snapshot. The patch with the effects on the client code can be seen here.

A couple of questions/consideration on the API, mostly from the user perspective:

  1. PluginManager#unloadPlugin
    • It is currently not clear why it could fail. Also, unlike in loadPlugin, it is not clear how the client can recover from that, and in what state the system remains (i.e., if I get an exception while trying to unload a plugin, does it mean the plugin is stuck there forever and the client simply can't unload it? Does it leave the plugin manager in a broken state?).
    • It seems that one of the sources of an exception is Plugin#stop. Do I understand it correctly that there are two classes of errors that are communicated with the same mechanism: (1) some errors occurring in the plugin manager; (2) any exceptions from the Plugin#stop — the code that the application does not control?
    • It returns a boolean flag ("true if the plugin was unloaded"). Shall the client code check both an exception and a flag? Do they communicate different things? If it is false, what shall the client do?
  2. PluginManager#startPlugin:
    • (similar to the 2nd question above) Is it only possible to get an exception because the Plugin#start throws, or can also some internal errors occur or precondition violations be communicated with this error?
    • This method returns a PluginState — shall I check it if the method returns successfully? Or is it always STARTED, in which case it might not be needed?

@decebals
Copy link
Member Author

decebals commented May 22, 2019

@dmitry-timofeev
Thanks that you tested the latest SNAPSHOT version!
Below I will try to answer your questions.

It is currently not clear why it could fail. Also, unlike in loadPlugin, it is not clear how the client can recover from that, and in what state the system remains (i.e., if I get an exception while trying to unload a plugin, does it mean the plugin is stuck there forever and the client simply can't unload it? Does it leave the plugin manager in a broken state?).

These modifications was made in the context of #292.
I'm almost sure that the client (user of PF4J) cannot recover from these exceptions. The idea with #292 was to surface the internal PluginExceptions. Until now, the exception was catched internally and an error is wrote in log.
Something like this:

try {
    // do something
} catch (PluginException e) {
    log.error(e.getMessage(). e);
}

The error in a method is signaled by the return value (null or other value - see PluginStatus).
The problem with the current approach is that you cannot catch and display the error message in your application (a nice error dialog). You see the error, only in the log file of your application.

Related to this subject, in one of our discussion about this subject I wrote:

Other discussion is what kind of exception we prefer: checked or unchecked (runtime) exception. Is it a good to create a PluginRuntimeException? For example PluginManager#loadPlugin will throws PluginException or PluginRuntimeException?

The dilemma remains in my mind: checked vs unchecked exceptions (both types of exceptions solve our goal). Take a look at JDBC API as an example. It is full with SQLException in the method signatures and in almost all cases you (as user) cannot recover.

It seems that one of the sources of an exception is Plugin#stop. Do I understand it correctly that there are two classes of errors that are communicated with the same mechanism: (1) some errors occurring in the plugin manager; (2) any exceptions from the Plugin#stop — the code that the application does not control?

I think that you understood correctly.
For example, try to comment throws PluginException from AbstractPluginManager#unloadPlugin
The error sources are:

  • a PluginException in stopPlugin method (the exception is propagated from Plugin#stop)
  • an IOException when we try to close the plugin ClassLoader

I think here that you want to display a nice error dialog in the application's UI when something goes wrong on a plugin unload action

It returns a boolean flag ("true if the plugin was unloaded"). Shall the client code check both an exception and a flag? Do they communicate different things? If it is false, what shall the client do?

Again, let's take a look in the code for AbstractPluginManager#unloadPlugin method. The method return true when the plugin has status STARTED after the method stopPlugin was called (it's an anomaly - something is wrong). Method stopPlugin can returns DISABLED (nothing to stop in this case).

(similar to the 2nd question above) Is it only possible to get an exception because the Plugin#start throws, or can also some internal errors occur or precondition violations be communicated with this error?

The startPlugin method is in pair with stopPlugin method (the same signature) for uniformity, with the same explanation (Plugin#stop method and enablePluginmethod can throw exception).

This method returns a PluginState — shall I check it if the method returns successfully? Or is it always STARTED, in which case it might not be needed?

In this situation, DISABLED (the plugin was disabled and it could not be automatically enabled) or CREATED (the plugin could not be resolved - a missing dependency). I recommend you to check both exception and returned value.

I explained to you how things work now (PF4J3), and I admit that something can be wrong and things can be improved.

@decebals
Copy link
Member Author

decebals commented May 24, 2019

So, it's time to take a decision about releasing or not PF4J 3 in this format.
It's time to vote with 👍 or 👎 on this comment. If you think that PF4J 3 is not prepared for release (your vote is 👎) please add a small comment about reason. Thanks!

@dmitry-timofeev
Copy link
Contributor

Hi, @decebals,

Thanks for the explanations!

Would it make sense to make PluginManager#stop and/or PluginManager#unload throw unchecked exceptions for unrecoverable conditions (e.g., an IOException when PF4J closes the plugin classloader)? In that case, if I understand correctly, only checked exceptions from Plugin#stop lifecycle method remain, if Plugins need to be able to communicate that to clients of PluginManagers.

We have a rather limited use-case: explicitly load/unload 3rd-party plugins with some extensions, no plugin dependencies, and a default Plugin, so we don't really expect any PluginException's originating from them. Hence I cannot consider all use-cases when this mechanism might be useful.

Also, would it make sense if the PluginManager#stop/#unload succeeded even if a certain plugin throws an exception in its Plugin#stop (i.e., PluginManager finished their job and threw a PluginException with all exceptions but the first originating from Plugin#stop as suppressed)?

@decebals
Copy link
Member Author

@dmitry-timofeev
I've been thinking all this week at this dilemma, checked vs unchecked exception. This is the single stop point to release PF4J 3 version.
In the end, I opted for the variant with unchecked version because they are very few situations when you can recover from a PluginException. In this context, I think that I will make PluginException to extends RuntimeException and I will document each method that can throws PluginException.
In this mode the API is more clear and if someone wants to catch the PluginException (unchecked exception) he can do it in a non intrusive mode.

@decebals
Copy link
Member Author

decebals commented Jun 8, 2019

With e81d905 I converted PluginException in PluginRuntimeException and use unchecked exceptions. In by opinion is better.
I published a new SNAPSHOT. Please test it.
If everything is OK, my intention is to release PF4J 3 asap.

@dmitry-timofeev
Copy link
Contributor

Thanks for the updates! I've upgraded the patch to the latest snapshot, our tests pass (they, however, cover only a small subset of the PF4J functionality).

@decebals decebals mentioned this issue Jun 12, 2019
@decebals
Copy link
Member Author

PF4J 3 released.
Thank you for your contributions!

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

No branches or pull requests

3 participants