Skip to content

Make project friendlier for Eclipse IDE development #11915

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
+64 −34
Draft

Make project friendlier for Eclipse IDE development #11915

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
26 changes: 20 additions & 6 deletions cluster-operator/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,26 @@
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-resources-plugin</artifactId>
<version>${maven.resources.version}</version>
<executions>
<execution>
<id>copy-kafka-versions</id>
<phase>generate-resources</phase>
<goals>
<goal>copy-resources</goal>
</goals>
<configuration>
<outputDirectory>${project.build.outputDirectory}</outputDirectory>
<resources>
<resource>
<directory>${strimziRootDirectory}</directory>
<includes>
<include>kafka-versions.yaml</include>
</includes>
</resource>
</resources>
</configuration>
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
Expand Down Expand Up @@ -365,12 +385,6 @@
<directory>src/main/resources-filtered</directory>
<filtering>true</filtering>
</resource>
<resource>
<directory>..</directory>
<includes>
<include>kafka-versions.yaml</include>
</includes>
</resource>
Copy link
Member

Choose a reason for hiding this comment

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

I am struggling to understand why it was removed from here but then having an execution task to copy it, IIUC

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Basically, it's due to this bug in Eclipse: eclipse-m2e/m2e-core#1790

Granted, it's Eclipse's bug, but nonetheless an impediment to using that IDE with Strimzi.

</resources>
</build>
</project>
10 changes: 6 additions & 4 deletions development-docs/DEV_GUIDE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ This document gives a detailed breakdown of the various build processes and opti
- [Build Pre-Requisites](#build-pre-requisites)
- [Using an IDE](#using-an-ide)
- [IntelliJ IDEA](#intellij-idea)
- [Eclipse](#eclipse)
- [IDE build problems](#ide-build-problems)
- [Build and deploy Strimzi from source](#build-and-deploy-from-source)
- [Build details](#build-details)
Expand All @@ -21,7 +22,7 @@ This document gives a detailed breakdown of the various build processes and opti
- [Helm Chart](#helm-chart)
- [Running system tests](#running-system-tests)
- [DCO Signoff](#dco-signoff)
- [Building container images for other platforms with Docker `buildx`](#building-container-images-for-other-platforms-with-docker-buildx)
- [Building container images for other platforms](#building-container-images-for-other-platforms)

<!-- /TOC -->

Expand Down Expand Up @@ -110,16 +111,17 @@ Afterwards IntelliJ should no longer have any `Cannot resolve symbol` errors.

Note: After running the Maven build in the terminal you might need to [reload the project](https://www.jetbrains.com/help/idea/delegate-build-and-run-actions-to-maven.html#maven_reimport) from the Maven tool window.

### Eclipse
* Eclipse users may find the [m2e-apt plugin](https://marketplace.eclipse.org/content/m2e-apt) useful for the automatic
configuration of Eclipse projects for annotation processing.

### IDE build problems

The build also uses a Java annotation processor. Some IDEs (such as IntelliJ's IDEA) by default don't run the annotation
processor in their build process. You can run `mvn clean install -DskipTests` to run the annotation processor
as part of the `maven` build, and the IDE should then be able to use the generated classes. It is also possible to
configure the IDE to run the annotation processor directly.

Eclipse users may find the [m2e-apt plugin](https://marketplace.eclipse.org/content/m2e-apt) useful for the automatic
configuration of Eclipse projects for annotation processing.

## Build and deploy from source

To build Strimzi from a source the operator and Kafka code needs to be compiled into container images and placed
Expand Down
38 changes: 26 additions & 12 deletions systemtest/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,32 @@

<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-resources-plugin</artifactId>
<version>${maven.resources.version}</version>
<executions>
<execution>
<id>copy-kafka-versions</id>
<phase>generate-resources</phase>
<goals>
<goal>copy-resources</goal>
</goals>
<configuration>
<outputDirectory>${project.build.outputDirectory}</outputDirectory>
<resources>
<resource>
<directory>${strimziRootDirectory}</directory>
<includes>
<include>kafka-versions.yaml</include>
<include>bridge.version</include>
</includes>
</resource>
</resources>
</configuration>
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-javadoc-plugin</artifactId>
Expand Down Expand Up @@ -329,18 +355,6 @@
</configuration>
</plugin>
</plugins>
<resources>
<resource>
<directory>src/main/resources</directory>
</resource>
<resource>
<directory>..</directory>
<includes>
<include>kafka-versions.yaml</include>
<include>bridge.version</include>
</includes>
</resource>
</resources>
</build>

<profiles>
Expand Down
24 changes: 12 additions & 12 deletions topic-operator/src/main/java/io/strimzi/operator/topic/KafkaHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,20 +142,20 @@ public PartitionedByError<ReconcilableTopic, TopicState> createTopics(List<Recon
var values = ctr.values();
return TopicOperatorUtil.partitionedByError(reconcilableTopics.stream().map(reconcilableTopic -> {
if (newTopicsErrors.containsKey(reconcilableTopic)) {
return new Pair<>(reconcilableTopic, Either.ofLeft(newTopicsErrors.get(reconcilableTopic)));
return new Pair<>(reconcilableTopic, Either.<TopicOperatorException, TopicState>ofLeft(newTopicsErrors.get(reconcilableTopic)));
Copy link
Member

Choose a reason for hiding this comment

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

why these changes? Does Eclipse fail the compile without them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it seems as though Eclipse JDT is not smart enough to determine the generic type arguments on these. I suspect the same errors would appear in VS Code, which at some point also used JDT.

Copy link
Member

Choose a reason for hiding this comment

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

The problem I have with this is that we can fix them now. Then we'll develop more but not using this syntax and then at some point someone using Eclipse will raise the issue. Is there really nothing that can't be done in Eclipse JDT to avoid this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's for sure an issue, but as I understand it the JDT compiler is a different implementation aiming for incremental compilation in the editor so it's always possible for these kind of differences. The generic use in this class is probably on the more complex end of the spectrum, so it may not be that common and it could just be dealt with if they occur again.

There are other occurrences of the type params being set already in this class, so it's not unprecedented. Apparently even javac was stunned by it at some point 😆

strimzi-kafka-operator/topic-operator/src/main/java/io/strimzi/operator/topic/KafkaHandler.java

Lines 193 to 195 in d311680

LOGGER.traceOp("Admin.listPartitionReassignments({}) failed with {}", apparentDifferentRfPartitions, e);
return apparentlyDifferentRfTopics.stream().map(pair ->
new Pair<>(pair.getKey(), Either.<TopicOperatorException, TopicState>ofLeft(handleAdminException(e)))).toList();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

FYI, I did find that Intellij can use the Eclipse compiler via Settings > Build, Execution, Deployment > Compiler > Java Compiler.

}
try {
values.get(reconcilableTopic.topicName()).get();
return new Pair<>(reconcilableTopic, Either.ofRight(
new TopicState(new TopicDescription(reconcilableTopic.topicName(),
return new Pair<>(reconcilableTopic, Either.<TopicOperatorException, TopicState>ofRight(
new TopicState(new TopicDescription(reconcilableTopic.topicName(),
false, List.of(), Set.of(), ctr.topicId(reconcilableTopic.topicName()).get()), null)
));
} catch (ExecutionException e) {
if (e.getCause() != null && e.getCause() instanceof TopicExistsException) {
// we treat this as a success, the next reconciliation checks the configuration
return new Pair<>(reconcilableTopic, Either.ofRight(null));
return new Pair<>(reconcilableTopic, Either.<TopicOperatorException, TopicState>ofRight(null));
} else {
return new Pair<>(reconcilableTopic, Either.ofLeft(handleAdminException(e)));
return new Pair<>(reconcilableTopic, Either.<TopicOperatorException, TopicState>ofLeft(handleAdminException(e)));
}
} catch (InterruptedException e) {
throw new UncheckedInterruptedException(e);
Expand Down Expand Up @@ -247,9 +247,9 @@ public PartitionedByError<ReconcilableTopic, Void> alterConfigs(List<Pair<Reconc
var alterConfigsResult = acr.values();
Stream<Pair<ReconcilableTopic, Either<TopicOperatorException, Void>>> entryStream = someAlterConfigs.stream().map(entry -> {
try {
return new Pair<>(entry.getKey(), Either.ofRight(alterConfigsResult.get(buildTopicConfigResource(entry.getKey().topicName())).get()));
return new Pair<>(entry.getKey(), Either.<TopicOperatorException, Void>ofRight(alterConfigsResult.get(buildTopicConfigResource(entry.getKey().topicName())).get()));
} catch (ExecutionException e) {
return new Pair<>(entry.getKey(), Either.ofLeft(handleAdminException(e)));
return new Pair<>(entry.getKey(), Either.<TopicOperatorException, Void>ofLeft(handleAdminException(e)));
} catch (InterruptedException e) {
throw new UncheckedInterruptedException(e);
}
Expand Down Expand Up @@ -359,9 +359,9 @@ public PartitionedByError<ReconcilableTopic, TopicState> describeTopics(List<Rec
throw new UncheckedInterruptedException(e);
}
if (exception != null) {
return new Pair<>(reconcilableTopic, Either.ofLeft(handleAdminException(exception)));
return new Pair<>(reconcilableTopic, Either.<TopicOperatorException, TopicState>ofLeft(handleAdminException(exception)));
} else {
return new Pair<>(reconcilableTopic, Either.ofRight(new TopicState(description, configs)));
return new Pair<>(reconcilableTopic, Either.<TopicOperatorException, TopicState>ofRight(new TopicState(description, configs)));
}
}));
}
Expand Down Expand Up @@ -398,12 +398,12 @@ public PartitionedByError<ReconcilableTopic, Object> deleteTopics(List<Reconcila
.map(reconcilableTopic -> {
try {
futuresMap.get(reconcilableTopic.topicName()).get();
return new Pair<>(reconcilableTopic, Either.ofRight(null));
return new Pair<>(reconcilableTopic, Either.<TopicOperatorException, Object>ofRight(null));
} catch (ExecutionException e) {
if (e.getCause() instanceof UnknownTopicOrPartitionException) {
return new Pair<>(reconcilableTopic, Either.ofRight(null));
return new Pair<>(reconcilableTopic, Either.<TopicOperatorException, Object>ofRight(null));
} else {
return new Pair<>(reconcilableTopic, Either.ofLeft(handleAdminException(e)));
return new Pair<>(reconcilableTopic, Either.<TopicOperatorException, Object>ofLeft(handleAdminException(e)));
}
} catch (InterruptedException e) {
throw new UncheckedInterruptedException(e);
Expand Down