-
-
Notifications
You must be signed in to change notification settings - Fork 8.8k
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
Extract interfaces for objects to be used through the executors widget #9749
Merged
Merged
Changes from 8 commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
74dedbf
Extract interfaces for objects to be used through the executors widget
Vlatombe 96d4f27
Fix Deprecated annotation
Vlatombe e879007
Missing @Override annotations
Vlatombe 877640d
Missing @since TODO
Vlatombe 32585b2
No longer true according to AbstractSubTask javadoc
Vlatombe 7118bc2
Restore old signature for compatibility
Vlatombe 1e715b1
Fix reviews
Vlatombe 4513cb0
Javadoc
Vlatombe ef81082
Typo
Vlatombe 742cee8
Fix inconsistency between hasOfflineCause and getOfflineCauseReason
Vlatombe 4cf4a80
Remove default implementation of ITask#getUrl
Vlatombe 91f0dd9
Provide default impl in SubTask for compatibility
Vlatombe b663199
Mark as CheckForNull
Vlatombe 19c6bf3
Spotbugs
Vlatombe f5bedaf
Merge branch 'master' into executors
Vlatombe e2c179a
ComputerSet#getComputers returns a collection that is sorted by name
Vlatombe ec3d4e7
Merge branch 'master' into executors
Vlatombe 516393a
Missing Override
Vlatombe 82adfa0
Move permission declaration to the right place, closer to usage
Vlatombe File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,7 +41,6 @@ | |
import hudson.console.AnnotatedLargeText; | ||
import hudson.init.Initializer; | ||
import hudson.model.Descriptor.FormException; | ||
import hudson.model.Queue.FlyweightTask; | ||
import hudson.model.labels.LabelAtom; | ||
import hudson.model.queue.WorkUnit; | ||
import hudson.node_monitors.AbstractDiskSpaceMonitor; | ||
|
@@ -106,6 +105,9 @@ | |
import java.util.logging.Logger; | ||
import java.util.regex.Matcher; | ||
import java.util.regex.Pattern; | ||
import jenkins.model.DisplayExecutor; | ||
import jenkins.model.IComputer; | ||
import jenkins.model.IDisplayExecutor; | ||
import jenkins.model.Jenkins; | ||
import jenkins.security.ImpersonatingExecutorService; | ||
import jenkins.security.MasterToSlaveCallable; | ||
|
@@ -116,8 +118,6 @@ | |
import jenkins.util.SystemProperties; | ||
import jenkins.widgets.HasWidgets; | ||
import net.jcip.annotations.GuardedBy; | ||
import org.jenkins.ui.icon.Icon; | ||
import org.jenkins.ui.icon.IconSet; | ||
import org.kohsuke.accmod.Restricted; | ||
import org.kohsuke.accmod.restrictions.DoNotUse; | ||
import org.kohsuke.accmod.restrictions.NoExternalUse; | ||
|
@@ -150,7 +150,7 @@ | |
* if a {@link Node} is configured (probably temporarily) with 0 executors, | ||
* you won't have a {@link Computer} object for it (except for the built-in node, | ||
* which always gets its {@link Computer} in case we have no static executors and | ||
* we need to run a {@link FlyweightTask} - see JENKINS-7291 for more discussion.) | ||
* we need to run a {@link Queue.FlyweightTask} - see JENKINS-7291 for more discussion.) | ||
* | ||
* Also, even if you remove a {@link Node}, it takes time for the corresponding | ||
* {@link Computer} to be removed, if some builds are already in progress on that | ||
|
@@ -164,7 +164,7 @@ | |
* @author Kohsuke Kawaguchi | ||
*/ | ||
@ExportedBean | ||
public /*transient*/ abstract class Computer extends Actionable implements AccessControlled, ExecutorListener, DescriptorByNameOwner, StaplerProxy, HasWidgets { | ||
public /*transient*/ abstract class Computer extends Actionable implements AccessControlled, IComputer, ExecutorListener, DescriptorByNameOwner, StaplerProxy, HasWidgets { | ||
|
||
private final CopyOnWriteArrayList<Executor> executors = new CopyOnWriteArrayList<>(); | ||
// TODO: | ||
|
@@ -351,12 +351,6 @@ public AnnotatedLargeText<Computer> getLogText() { | |
return new AnnotatedLargeText<>(getLogFile(), Charset.defaultCharset(), false, this); | ||
} | ||
|
||
@NonNull | ||
@Override | ||
public ACL getACL() { | ||
return Jenkins.get().getAuthorizationStrategy().getACL(this); | ||
} | ||
|
||
/** | ||
* If the computer was offline (either temporarily or not), | ||
* this method will return the cause. | ||
|
@@ -369,14 +363,13 @@ public OfflineCause getOfflineCause() { | |
return offlineCause; | ||
} | ||
|
||
/** | ||
* If the computer was offline (either temporarily or not), | ||
* this method will return the cause as a string (without user info). | ||
* | ||
* @return | ||
* empty string if the system was put offline without given a cause. | ||
*/ | ||
@Override | ||
public boolean hasOfflineCause() { | ||
return offlineCause != null; | ||
} | ||
|
||
@Exported | ||
@Override | ||
public String getOfflineCauseReason() { | ||
if (offlineCause == null) { | ||
return ""; | ||
|
@@ -581,9 +574,6 @@ public int getNumExecutors() { | |
return numExecutors; | ||
} | ||
|
||
/** | ||
* Returns {@link Node#getNodeName() the name of the node}. | ||
*/ | ||
public @NonNull String getName() { | ||
return nodeName != null ? nodeName : ""; | ||
} | ||
|
@@ -628,6 +618,7 @@ public BuildTimelineWidget getTimeline() { | |
} | ||
|
||
@Exported | ||
@Override | ||
public boolean isOffline() { | ||
return temporarilyOffline || getChannel() == null; | ||
} | ||
|
@@ -645,12 +636,6 @@ public boolean isManualLaunchAllowed() { | |
return getRetentionStrategy().isManualLaunchAllowed(this); | ||
} | ||
|
||
|
||
/** | ||
* Is a {@link #connect(boolean)} operation in progress? | ||
*/ | ||
public abstract boolean isConnecting(); | ||
|
||
/** | ||
* Returns true if this computer is supposed to be launched via inbound protocol. | ||
* @deprecated since 2008-05-18. | ||
|
@@ -662,14 +647,8 @@ public boolean isJnlpAgent() { | |
return false; | ||
} | ||
|
||
/** | ||
* Returns true if this computer can be launched by Hudson proactively and automatically. | ||
* | ||
* <p> | ||
* For example, inbound agents return {@code false} from this, because the launch process | ||
* needs to be initiated from the agent side. | ||
*/ | ||
@Exported | ||
@Override | ||
public boolean isLaunchSupported() { | ||
return true; | ||
} | ||
|
@@ -727,14 +706,8 @@ public void setTemporarilyOffline(boolean temporarilyOffline, OfflineCause cause | |
} | ||
} | ||
|
||
/** | ||
* Returns the icon for this computer. | ||
* | ||
* It is both the recommended and default implementation to serve different icons based on {@link #isOffline} | ||
* | ||
* @see #getIconClassName() | ||
*/ | ||
@Exported | ||
@Override | ||
public String getIcon() { | ||
// The machine was taken offline by someone | ||
if (isTemporarilyOffline() && getOfflineCause() instanceof OfflineCause.UserCause) return "symbol-computer-disconnected"; | ||
|
@@ -748,19 +721,14 @@ public String getIcon() { | |
} | ||
|
||
/** | ||
* Returns the class name that will be used to lookup the icon. | ||
* | ||
* This class name will be added as a class tag to the html img tags where the icon should | ||
* show up followed by a size specifier given by {@link Icon#toNormalizedIconSizeClass(String)} | ||
* The conversion of class tag to src tag is registered through {@link IconSet#addIcon(Icon)} | ||
* | ||
* It is both the recommended and default implementation to serve different icons based on {@link #isOffline} | ||
* {@inheritDoc} | ||
* | ||
* @see #getIcon() | ||
* <p> | ||
* It is both the recommended and default implementation to serve different icons based on {@link #isOffline}. | ||
*/ | ||
@Exported | ||
public String getIconClassName() { | ||
return getIcon(); | ||
return IComputer.super.getIconClassName(); | ||
} | ||
|
||
public String getIconAltText() { | ||
|
@@ -780,6 +748,8 @@ public String getCaption() { | |
return Messages.Computer_Caption(nodeName); | ||
} | ||
|
||
@Override | ||
@NonNull | ||
public String getUrl() { | ||
return "computer/" + Util.fullEncode(getName()) + "/"; | ||
} | ||
|
@@ -947,19 +917,18 @@ public int countIdle() { | |
return n; | ||
} | ||
|
||
/** | ||
* Returns the number of {@link Executor}s that are doing some work right now. | ||
*/ | ||
@Override | ||
public final int countBusy() { | ||
return countExecutors() - countIdle(); | ||
} | ||
|
||
/** | ||
* Returns the current size of the executor pool for this computer. | ||
* {@inheritDoc} | ||
* This number may temporarily differ from {@link #getNumExecutors()} if there | ||
* are busy tasks when the configured size is decreased. OneOffExecutors are | ||
* not included in this count. | ||
*/ | ||
@Override | ||
public final int countExecutors() { | ||
return executors.size(); | ||
} | ||
|
@@ -996,14 +965,14 @@ public List<Executor> getAllExecutors() { | |
} | ||
|
||
/** | ||
* Used to render the list of executors. | ||
* @return a snapshot of the executor display information | ||
* {@inheritDoc} | ||
* @since 1.607 | ||
*/ | ||
@Restricted(NoExternalUse.class) | ||
public List<DisplayExecutor> getDisplayExecutors() { | ||
@Override | ||
@NonNull | ||
public List<IDisplayExecutor> getDisplayExecutors() { | ||
// The size may change while we are populating, but let's start with a reasonable guess to minimize resizing | ||
List<DisplayExecutor> result = new ArrayList<>(executors.size() + oneOffExecutors.size()); | ||
List<IDisplayExecutor> result = new ArrayList<>(executors.size() + oneOffExecutors.size()); | ||
int index = 0; | ||
for (Executor e : executors) { | ||
if (e.isDisplayCell()) { | ||
|
@@ -1659,15 +1628,8 @@ public Object getTarget() { | |
return e != null ? e.getOwner() : null; | ||
} | ||
|
||
/** | ||
* Returns {@code true} if the computer is accepting tasks. Needed to allow agents programmatic suspension of task | ||
* scheduling that does not overlap with being offline. | ||
* | ||
* @return {@code true} if the computer is accepting tasks | ||
* @see hudson.slaves.RetentionStrategy#isAcceptingTasks(Computer) | ||
* @see hudson.model.Node#isAcceptingTasks() | ||
*/ | ||
@OverrideMustInvoke | ||
@Override | ||
public boolean isAcceptingTasks() { | ||
final Node node = getNode(); | ||
return getRetentionStrategy().isAcceptingTasks(this) && (node == null || node.isAcceptingTasks()); | ||
|
@@ -1727,79 +1689,12 @@ public static void relocateOldLogs() { | |
} | ||
} | ||
|
||
/** | ||
* A value class to provide a consistent snapshot view of the state of an executor to avoid race conditions | ||
* during rendering of the executors list. | ||
* | ||
* @since 1.607 | ||
*/ | ||
@Restricted(NoExternalUse.class) | ||
public static class DisplayExecutor implements ModelObject { | ||
Comment on lines
-1730
to
-1737
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. Moved to its own file. |
||
|
||
@NonNull | ||
private final String displayName; | ||
@NonNull | ||
private final String url; | ||
@NonNull | ||
private final Executor executor; | ||
|
||
public DisplayExecutor(@NonNull String displayName, @NonNull String url, @NonNull Executor executor) { | ||
this.displayName = displayName; | ||
this.url = url; | ||
this.executor = executor; | ||
} | ||
|
||
@Override | ||
@NonNull | ||
public String getDisplayName() { | ||
return displayName; | ||
} | ||
|
||
@NonNull | ||
public String getUrl() { | ||
return url; | ||
} | ||
|
||
@NonNull | ||
public Executor getExecutor() { | ||
return executor; | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
String sb = "DisplayExecutor{" + "displayName='" + displayName + '\'' + | ||
", url='" + url + '\'' + | ||
", executor=" + executor + | ||
'}'; | ||
return sb; | ||
} | ||
|
||
@Override | ||
public boolean equals(Object o) { | ||
if (this == o) { | ||
return true; | ||
} | ||
if (o == null || getClass() != o.getClass()) { | ||
return false; | ||
} | ||
|
||
DisplayExecutor that = (DisplayExecutor) o; | ||
|
||
return executor.equals(that.executor); | ||
} | ||
|
||
@Extension(ordinal = Double.MAX_VALUE) | ||
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. (ignore WS) |
||
@Restricted(DoNotUse.class) | ||
public static class InternalComputerListener extends ComputerListener { | ||
@Override | ||
public void onOnline(Computer c, TaskListener listener) throws IOException, InterruptedException { | ||
c.cachedEnvironment = null; | ||
} | ||
} | ||
|
||
@Extension(ordinal = Double.MAX_VALUE) | ||
@Restricted(DoNotUse.class) | ||
public static class InternalComputerListener extends ComputerListener { | ||
@Override | ||
public int hashCode() { | ||
return executor.hashCode(); | ||
public void onOnline(Computer c, TaskListener listener) throws IOException, InterruptedException { | ||
c.cachedEnvironment = null; | ||
} | ||
} | ||
|
||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Only here for
@Exported