[MRESOLVER-302] Introduce onSessionClose hooks#232
[MRESOLVER-302] Introduce onSessionClose hooks#232cstamas wants to merge 2 commits intoapache:masterfrom
Conversation
That integrators must integrate, to provide onSessionClosed hooks functionality to any component requiring it. --- https://issues.apache.org/jira/browse/MRESOLVER-302
| * @param session the session that just ended, never {@code null}. | ||
| * @since TBD | ||
| */ | ||
| void sessionEnded( RepositorySystemSession session ); |
There was a problem hiding this comment.
From a consumer perspective a close() method on RSS would be a lot easier and in addition allows to use
- try with resources
- semantic code analysis tools to emit a WARN when someone forgot to call close.
There was a problem hiding this comment.
This is the least painful way with 1.x. Problem is how session instances are created (by client code, using def ctor of DefaultRepositorySystemSession, no factory for it) and derived. Just look thru Maven and plugin sources as example. They are not "boxed" nor are hierarchical, you can get session A and have it "derived" (using copy ctor) into session B and then continue to use it (typical when local repo is overridden in plugins), etc. And then i did not even mention "forwarding" session...
So yes, ideally session would be:
- created by some factory (repo system, ideally?)
- closeable
But doing this would be very disruptive, so baby steps... leave these changes for 2.0
There was a problem hiding this comment.
Understood, but the same uncertainty is now applicable to sessionEnded(...). When should consumers call it? What about sessions created with https://maven.apache.org/resolver/apidocs/org/eclipse/aether/DefaultRepositorySystemSession.html#%3Cinit%3E(org.eclipse.aether.RepositorySystemSession)? What about forwarding sessions. The Javadocs need to be extended in this regard.
There was a problem hiding this comment.
Given all that I still think that a close() on RepositorySystemSession is preferable.
The different implementations need to implement that method differently:
DefaultRepositorySystemSession.close()should really do the cleanup in case it has been created from scratchDefaultRepositorySystemSession.close()should be a noop in case it has been created via copy constructorAbstractForwardingRepositorySystemSession.close()should be a noop
Doing this should be fully backwards compatible and still expose an API which is much easier to digest.
| * <p> | ||
| * <en>Same session instance can be started only once.</em> | ||
| * | ||
| * @param session the session that is about to start, never {@code null}. |
There was a problem hiding this comment.
Why can’t this be transparently handled whenever RSS is created?
|
Superseded by #357 |
|
Resolve #974 |
1 similar comment
|
Resolve #974 |
3 participants
That integrators must integrate, to provide onSessionClosed hooks functionality to any component requiring it.
https://issues.apache.org/jira/browse/MRESOLVER-302