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

Patch class loader #64

Merged
merged 2 commits into from Aug 24, 2020
Merged

Conversation

loghijiaha
Copy link

JENKINS-63475 - Shading zeppelin classpath as zeppelin2 as suggested here https://www.jenkins.io/doc/developer/plugin-development/dependencies-and-class-loading/#pluginfirstclassloader-and-its-discontents

Checklist

  • I have referenced the Jira issue related to my changes in one or more commit messages
  • Unit tests pass locally with my changes
  • I have added documentation as necessary
  • No Javadoc warnings were introduced with my changes
  • No spotbugs warnings were introduced with my changes
  • I have interactively tested my changes

Types of changes

  • Infrastructure change (non-breaking change which updates dependencies or improves infrastructure)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

@loghijiaha loghijiaha requested a review from kinow August 24, 2020 03:25
* THE SOFTWARE.
*/

package org.apache.zeppelin2.jupyter;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@loghijiaha did you test with this package?

I thought we had to match the fully qualified name of the class, including package + class, and I think they are not using zeppelin2 in Zeppelin? https://github.com/apache/zeppelin/blob/master/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/util/ProcessLauncher.java

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried pluginFirstclass loader configured in the pom.xml. That does not work. Then tried this it worked pretty well.

import org.apache.zeppelin.resource.LocalResourcePool;
import org.apache.zeppelin.resource.ResourcePool;
import org.apache.zeppelin2.jupyter.JupyterInterpreter;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here I am using the 2nd version. So it works as we expected.

Copy link
Member

@kinow kinow Aug 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If that's the case, we might as well move that to our packages, under io.jenkins.plugins.ml. WDYT? (we still need to leave the license to AL)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay @kinow we can move into out packages and set the licence to AL. 👍

@kinow
Copy link
Member

kinow commented Aug 24, 2020

Time to review it! Finished $work, just pushing commits/pull requests. Then prepare dinner, work out, and will test & review this one before calling it a day @loghijiaha ! Thanks for updating it so quickly!

@loghijiaha
Copy link
Author

Well sheduled agenda +1 @kinow. Thanks 🎉

Moved some classes to io.jenkins.plugins.ml

Changed the timeout to second
Copy link
Member

@kinow kinow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Downloaded Jenkins LTS 2.235.5, and started it with JENKINS_HOME=/tmp/jenkins java -jar jenkins.war. Installed the selected plug-ins (which took a long time), and also manually installed the hpi.

To generate the plugin hpi, I checked out the branch

kinow@ranma:~/Development/java/jenkins/machine-learning-plugin$ git log -n 1
commit 524af9223a34c757f712e0fa2a64394eee274ad3 (HEAD -> pr-64)
Author: loghi <loghijiaha.16@cse.mrt.ac.lk>
Date:   Mon Aug 24 08:18:08 2020 +0530

    Added shaded interpreter class
    
    Moved some classes to io.jenkins.plugins.ml
    
    Changed the timeout to second

and ran mvn clean install, producing ./target/machine-learning.hpi. Then time to restart it. First activated my Python virtual environment, then started Jenkins again.

Went to manage Jenkins, and added the javascript kernel. Hitting "Test connection", it said "Connection Successful", even though there were some console logs (may need a look later, no blockers for this PR IMO).

2020-08-24 10:11:35.741+0000 [id=86]	SEVERE	o.a.z.j.JupyterKernelClient$1#onError: Fail to call IPython grpc
io.grpc.StatusRuntimeException: UNKNOWN: Exception iterating responses: 'payload'
	at io.grpc.Status.asRuntimeException(Status.java:526)
	at io.grpc.stub.ClientCalls$StreamObserverToCallListenerAdapter.onClose(ClientCalls.java:434)
	at io.grpc.PartialForwardingClientCallListener.onClose(PartialForwardingClientCallListener.java:39)
	at io.grpc.ForwardingClientCallListener.onClose(ForwardingClientCallListener.java:23)
	at io.grpc.ForwardingClientCallListener$SimpleForwardingClientCallListener.onClose(ForwardingClientCallListener.java:40)
	at io.grpc.internal.CensusStatsModule$StatsClientInterceptor$1$1.onClose(CensusStatsModule.java:678)
	at io.grpc.PartialForwardingClientCallListener.onClose(PartialForwardingClientCallListener.java:39)
	at io.grpc.ForwardingClientCallListener.onClose(ForwardingClientCallListener.java:23)
	at io.grpc.ForwardingClientCallListener$SimpleForwardingClientCallListener.onClose(ForwardingClientCallListener.java:40)
	at io.grpc.internal.CensusTracingModule$TracingClientInterceptor$1$1.onClose(CensusTracingModule.java:403)
	at io.grpc.internal.ClientCallImpl.closeObserver(ClientCallImpl.java:459)
	at io.grpc.internal.ClientCallImpl.access$300(ClientCallImpl.java:63)
	at io.grpc.internal.ClientCallImpl$ClientStreamListenerImpl.close(ClientCallImpl.java:546)
	at io.grpc.internal.ClientCallImpl$ClientStreamListenerImpl.access$600(ClientCallImpl.java:467)
	at io.grpc.internal.ClientCallImpl$ClientStreamListenerImpl$1StreamClosed.runInContext(ClientCallImpl.java:584)
	at io.grpc.internal.ContextRunnable.run(ContextRunnable.java:37)
	at io.grpc.internal.SerializingExecutor.run(SerializingExecutor.java:123)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)
2020-08-24 10:11:36.752+0000 [id=51]	INFO	i.j.p.m.j.JupyterKernelInterpreter#close: Shutdown Jupyter Kernel Process
2020-08-24 10:11:41.771+0000 [id=51]	INFO	i.j.p.m.j.JupyterKernelInterpreter#close: Jupyter Kernel is killed
2020-08-24 10:11:41.785+0000 [id=86]	SEVERE	i.g.internal.SerializingExecutor#run: Exception while executing runnable io.grpc.internal.ClientCallImpl$ClientStreamListenerImpl$1StreamClosed@13dcaf4f
java.lang.NullPointerException
	at io.grpc.stub.ClientCalls$StreamObserverToCallListenerAdapter.onClose(ClientCalls.java:434)
	at io.grpc.PartialForwardingClientCallListener.onClose(PartialForwardingClientCallListener.java:39)
	at io.grpc.ForwardingClientCallListener.onClose(ForwardingClientCallListener.java:23)
	at io.grpc.ForwardingClientCallListener$SimpleForwardingClientCallListener.onClose(ForwardingClientCallListener.java:40)
	at io.grpc.internal.CensusStatsModule$StatsClientInterceptor$1$1.onClose(CensusStatsModule.java:678)
	at io.grpc.PartialForwardingClientCallListener.onClose(PartialForwardingClientCallListener.java:39)
	at io.grpc.ForwardingClientCallListener.onClose(ForwardingClientCallListener.java:23)
	at io.grpc.ForwardingClientCallListener$SimpleForwardingClientCallListener.onClose(ForwardingClientCallListener.java:40)
	at io.grpc.internal.CensusTracingModule$TracingClientInterceptor$1$1.onClose(CensusTracingModule.java:403)
	at io.grpc.internal.ClientCallImpl.closeObserver(ClientCallImpl.java:459)
	at io.grpc.internal.ClientCallImpl.access$300(ClientCallImpl.java:63)
	at io.grpc.internal.ClientCallImpl$ClientStreamListenerImpl.close(ClientCallImpl.java:546)
	at io.grpc.internal.ClientCallImpl$ClientStreamListenerImpl.access$600(ClientCallImpl.java:467)
	at io.grpc.internal.ClientCallImpl$ClientStreamListenerImpl$1StreamClosed.runInContext(ClientCallImpl.java:584)
	at io.grpc.internal.ContextRunnable.run(ContextRunnable.java:37)
	at io.grpc.internal.SerializingExecutor.run(SerializingExecutor.java:123)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)

Then added a simple job to test the plug-in. It was configured with one JS code block, and another Python code block.

#JS code block in one build step
console.log('oi')

#Py code block in another build step
print(f'{1 + 2}')

Executing it, everything worked fine. And no errors in the console logs!

image

Finally, modified the scripts to print the working directory.

#JS code block in one build step
console.log('oi')

#Py code block in another build step
print(f'{1 + 2}')

image

The working directory looks right to me.

Well done Loghi!

@loghijiaha
Copy link
Author

Yes @kinow I also noticed during the last checking. Because we disabling grpc in builder class. Need a fix in the next release. 👍

@loghijiaha
Copy link
Author

Thank you for taking your time and patience to test this:tada:

@loghijiaha loghijiaha merged commit 40eb43d into jenkinsci:master Aug 24, 2020
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

Successfully merging this pull request may close these issues.

None yet

2 participants