Skip to content

[SPARK-49156] Enforce ImmutableField and UselessParentheses rules #37

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.

Sign up for GitHub

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

Closed
williamhyun wants to merge 2 commits into from
Closed

[SPARK-49156] Enforce ImmutableField and UselessParentheses rules #37

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8595d00
Immutable field
williamhyun Aug 8, 2024
2f1e1b5
Useless parenthesis
williamhyun Aug 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions config/pmd/ruleset.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
<exclude name="ShortVariable" />
<exclude name="TooManyStaticImports" />
<exclude name="UseUnderscoresInNumericLiterals" />
<exclude name="UselessParentheses" />
</rule>
<rule ref="category/java/design.xml">
<exclude name="AbstractClassWithoutAnyMethod" />
Expand All @@ -51,7 +50,6 @@
<exclude name="CognitiveComplexity" />
<exclude name="CyclomaticComplexity" />
<exclude name="ExcessiveImports" />
<exclude name="ImmutableField" />
<exclude name="LawOfDemeter" />
<exclude name="NPathComplexity" />
<exclude name="SignatureDeclareThrowsException" />
Expand Down
8 changes: 4 additions & 4 deletions spark-operator-api/src/main/java/org/apache/spark/k8s/operator/status/ApplicationStatus.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@ public ApplicationStatus terminateOrRestart(
ApplicationState state =
new ApplicationState(ApplicationStateSummary.ResourceReleased, stateMessageOverride);
if (ResourceRetainPolicy.Always.equals(resourceRetainPolicy)
|| (ResourceRetainPolicy.OnFailure.equals(resourceRetainPolicy)
&& currentState.currentStateSummary.isFailure())) {
|| ResourceRetainPolicy.OnFailure.equals(resourceRetainPolicy)
&& currentState.currentStateSummary.isFailure()) {
state = terminateAppWithoutReleaseResource(stateMessageOverride);
}
return new ApplicationStatus(
Expand All @@ -116,8 +116,8 @@ public ApplicationStatus terminateOrRestart(
ApplicationState state =
new ApplicationState(ApplicationStateSummary.ResourceReleased, stateMessage);
if (ResourceRetainPolicy.Always.equals(resourceRetainPolicy)
|| (ResourceRetainPolicy.OnFailure.equals(resourceRetainPolicy)
&& currentState.currentStateSummary.isFailure())) {
|| ResourceRetainPolicy.OnFailure.equals(resourceRetainPolicy)
&& currentState.currentStateSummary.isFailure()) {
state = terminateAppWithoutReleaseResource(stateMessage);
}
// still use previous & current attempt summary - they are to be updated only upon
Expand Down
2 changes: 1 addition & 1 deletion ...ator/src/main/java/org/apache/spark/k8s/operator/metrics/healthcheck/SentinelManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public static boolean isSentinelResource(HasMetadata resource) {
}

private static boolean shouldSentinelWatchGivenNamespace(String namespace) {
if ((!Utils.getWatchedNamespaces().isEmpty())
if (!Utils.getWatchedNamespaces().isEmpty()
&& !Utils.getWatchedNamespaces().contains(namespace)) {
if (log.isErrorEnabled()) {
log.error("Skip watching sentinel resource in namespace {}", namespace);
Expand Down
2 changes: 1 addition & 1 deletion ...src/main/java/org/apache/spark/k8s/operator/reconciler/reconcilesteps/AppRunningStep.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public ReconcileProgress reconcile(
String stateMessage = context.getResource().getStatus().getCurrentState().getMessage();
if (instanceConfig == null
|| instanceConfig.getInitExecutors() == 0L
|| (!prevStateSummary.isStarting() && instanceConfig.getMinExecutors() == 0L)) {
|| !prevStateSummary.isStarting() && instanceConfig.getMinExecutors() == 0L) {
proposedStateSummary = ApplicationStateSummary.RunningHealthy;
stateMessage = Constants.RUNNING_HEALTHY_MESSAGE;
} else {
Expand Down
2 changes: 1 addition & 1 deletion spark-operator/src/main/java/org/apache/spark/k8s/operator/utils/StatusRecorder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ private void patchAndStatusWithVersionLocked(CR resource, KubernetesClient clien
resource.getMetadata().setResourceVersion(updated.getMetadata().getResourceVersion());
err = null;
} catch (KubernetesClientException e) {
log.error("Error while patching status, retrying {}/{}...", (i + 1), maxRetry, e);
log.error("Error while patching status, retrying {}/{}...", i + 1, maxRetry, e);
Thread.sleep(TimeUnit.SECONDS.toMillis(API_RETRY_ATTEMPT_AFTER_SECONDS.getValue()));
err = e;
}
Expand Down
4 changes: 2 additions & 2 deletions spark-operator/src/test/java/org/apache/spark/k8s/operator/probe/HealthProbeTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,9 @@ class HealthProbeTest {
private KubernetesClient kubernetesClient;
private AtomicBoolean isRunning;
private AtomicBoolean isRunning2;
private Map<String, Map<String, InformerWrappingEventSourceHealthIndicator>>
private final Map<String, Map<String, InformerWrappingEventSourceHealthIndicator>>
unhealthyEventSources = new HashMap<>();
private Map<String, Map<String, InformerWrappingEventSourceHealthIndicator>>
private final Map<String, Map<String, InformerWrappingEventSourceHealthIndicator>>
unhealthyEventSources2 = new HashMap<>();

@BeforeAll
Expand Down
12 changes: 6 additions & 6 deletions ...erator/src/test/java/org/apache/spark/k8s/operator/reconciler/SparkAppReconcilerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,12 @@
import org.apache.spark.k8s.operator.utils.SparkAppStatusRecorder;

class SparkAppReconcilerTest {
private SparkAppStatusRecorder mockRecorder = mock(SparkAppStatusRecorder.class);
private SentinelManager<SparkApplication> mockSentinelManager = mock(SentinelManager.class);
private KubernetesClient mockClient = mock(KubernetesClient.class);
private Context<SparkApplication> mockContext = mock(Context.class);
private Pod mockDriver = mock(Pod.class);
private SparkAppSubmissionWorker mockWorker = mock(SparkAppSubmissionWorker.class);
private final SparkAppStatusRecorder mockRecorder = mock(SparkAppStatusRecorder.class);
private final SentinelManager<SparkApplication> mockSentinelManager = mock(SentinelManager.class);
private final KubernetesClient mockClient = mock(KubernetesClient.class);
private final Context<SparkApplication> mockContext = mock(Context.class);
private final Pod mockDriver = mock(Pod.class);
private final SparkAppSubmissionWorker mockWorker = mock(SparkAppSubmissionWorker.class);
SparkApplication app = new SparkApplication();
SparkAppReconciler reconciler =
new SparkAppReconciler(mockWorker, mockRecorder, mockSentinelManager);
Expand Down