-
-
Notifications
You must be signed in to change notification settings - Fork 8.6k
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
[JENKINS-43496] - Add handling of the null Node#createComputer() result. #2922
Changes from 1 commit
ac43b32
de7bd15
7d70466
f97c3d6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,6 +40,7 @@ | |
import hudson.security.ACL; | ||
import hudson.security.AccessControlled; | ||
import hudson.security.Permission; | ||
import hudson.slaves.Cloud; | ||
import hudson.slaves.ComputerListener; | ||
import hudson.slaves.NodeDescriptor; | ||
import hudson.slaves.NodeProperty; | ||
|
@@ -65,6 +66,8 @@ | |
import net.sf.json.JSONObject; | ||
import org.acegisecurity.Authentication; | ||
import org.jvnet.localizer.Localizable; | ||
import org.kohsuke.accmod.Restricted; | ||
import org.kohsuke.accmod.restrictions.NoExternalUse; | ||
import org.kohsuke.stapler.BindInterceptor; | ||
import org.kohsuke.stapler.Stapler; | ||
import org.kohsuke.stapler.StaplerRequest; | ||
|
@@ -214,8 +217,13 @@ public final VirtualChannel getChannel() { | |
|
||
/** | ||
* Creates a new {@link Computer} object that acts as the UI peer of this {@link Node}. | ||
* | ||
* Nobody but {@link Jenkins#updateComputerList()} should call this method. | ||
* @return Created instance of the computer. | ||
* Can be {@code null} if the {@link Node} implementation does not support it (e.g. {@link Cloud} agent). | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this actually true? A There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed. |
||
*/ | ||
@CheckForNull | ||
@Restricted(NoExternalUse.class) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Won't this cause compilation failures for sub-classes? (and if it doesn't then won't the subclass method be invocable anyway, in which case the annotation is useless There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @stephenc @jglick My intention was to reflect the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @oleg-nenashev ah, were you looking for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jglick Yep, it should work better in this case |
||
protected abstract Computer createComputer(); | ||
|
||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -306,6 +306,8 @@ public Callable<ClockDifference,IOException> getClockDifferenceCallable() { | |
return new GetClockDifference1(); | ||
} | ||
|
||
@Override | ||
@Nonnull | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not true in the case of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree. Better to just remove since the parent Javadoc describes the |
||
public Computer createComputer() { | ||
return new SlaveComputer(this); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
… has no … update its …