Skip to content

MINOR: Update build and test dependencies#9645

Merged
ijuma merged 8 commits intoapache:trunkfrom
ijuma:build-test-dep-updates
Nov 24, 2020
Merged

MINOR: Update build and test dependencies#9645
ijuma merged 8 commits intoapache:trunkfrom
ijuma:build-test-dep-updates

Conversation

@ijuma
Copy link
Copy Markdown
Member

@ijuma ijuma commented Nov 23, 2020 •
edited
Loading

The spotbugs upgrade means we can re-enable
RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE and RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE.
These uncovered one bug, one unnecessary null check and one
false positive. Addressed them all, including a test for the bug.

  • gradle (6.7.0 -> 6.7.1): minor fixes.
  • gradle versions plugin (0.29.0 -> 0.36.0): minor fixes.
  • grgit (4.0.2 -> 4.1.0): a few small fixes and dependency bumps.
  • owasp dependency checker plugin (5.3.2.1 -> 6.0.3): improved db
    schema, data and several fixes.
  • scoverage plugin (4.0.2 -> 5.0.0): support Scala 2.13.
  • shadow plugin (6.0.0 -> 6.1.0): require Java 8, support for Java 16.
  • spotbugs plugin (4.4.4 -> 4.6.0): support SARIF reporting standard.
  • spotbugs (4.0.6 -> 4.1.4): support for Java 16 and various fixes including
    try with resources false positive.
  • spotless plugin (5.1.0 -> 5.8.2): minor fixes.
  • test retry plugin (1.1.6 -> 1.1.9): newer gradle and java version compatibility
    fixes.
  • mockito (3.5.7 -> 3.6.0): minor fixes.
  • powermock (2.0.7 -> 2.0.9): minor fixes.

Release notes links:

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

ijuma
ijuma commented Nov 23, 2020
View reviewed changes
Comment thread clients/src/main/java/org/apache/kafka/common/header/internals/RecordHeader.java
ijuma
ijuma commented Nov 23, 2020
View reviewed changes
Comment thread clients/src/main/java/org/apache/kafka/clients/admin/ScramMechanism.java
return Arrays.stream(VALUES)
.filter(mechanism -> mechanism.name().equals(normalizedMechanism))
.findFirst()
.orElse(UNKNOWN);
Copy link
Copy Markdown
Member Author

@ijuma ijuma Nov 23, 2020

Choose a reason for hiding this comment

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

@rondagostino looks like the original implementation didn't achieve the goal. I added a test that failed without this change.

Copy link
Copy Markdown
Contributor

@rondagostino rondagostino Nov 23, 2020

Choose a reason for hiding this comment

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

Ah, you are right: valueOf() Throws: IllegalArgumentException - if the specified enum type has no constant with the specified name, or the specified class object does not represent an enum type.

Thanks for catching/testing for/fixing it.

Copy link
Copy Markdown
Member Author

@ijuma ijuma Nov 24, 2020

Choose a reason for hiding this comment

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

@rondagostino do we need to cherry-pick this change to the 2.7 branch? I am not sure under which context this method is used and whether it's important enough to backport.

@ijuma ijuma marked this pull request as ready for review November 23, 2020 22:23
@ijuma ijuma requested a review from chia7712 November 23, 2020 22:31
chia7712
chia7712 approved these changes Nov 24, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

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

overall +1 and one small comment is left.

Comment thread clients/src/main/java/org/apache/kafka/clients/admin/ScramMechanism.java Outdated
public static ScramMechanism fromMechanismName(String mechanismName) {
ScramMechanism retvalFoundMechanism = ScramMechanism.valueOf(mechanismName.replace('-', '_'));
return retvalFoundMechanism != null ? retvalFoundMechanism : UNKNOWN;
String normalizedMechanism = mechanismName.replace('-', '_');
Copy link
Copy Markdown
Member

@chia7712 chia7712 Nov 24, 2020

Choose a reason for hiding this comment

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

Could we reuse mechanismName (https://github.com/apache/kafka/blob/trunk/clients/src/main/java/org/apache/kafka/clients/admin/ScramMechanism.java#L79) so we don't need this duplicate replacement.

Copy link
Copy Markdown
Member Author

@ijuma ijuma Nov 24, 2020

Choose a reason for hiding this comment

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

Good suggestion, applied it.

@ijuma
Copy link
Copy Markdown
Member Author

ijuma commented Nov 24, 2020

JDK 8 and 15 builds passed. JDK 11 had one flaky unrelated failure:

org.apache.kafka.streams.integration.StreamTableJoinTopologyOptimizationIntegrationTest.shouldDoStreamTableJoinWithDifferentNumberOfPartitions[Optimization = all]

@ijuma ijuma merged commit a5986bd into apache:trunk Nov 24, 2020
@ijuma ijuma deleted the build-test-dep-updates branch November 24, 2020 14:20
ijuma added a commit to ijuma/kafka that referenced this pull request Dec 2, 2020
@ijuma
Merge remote-tracking branch 'apache-github/trunk' into skip-to-struc…
d834f14 
…t-for-generated-requests

* apache-github/trunk: (405 commits)
KAFKA-6687: restrict DSL to allow only Streams from the same source
topics (apache#9609)
  MINOR: Small cleanups in `AlterIsr` handling logic (apache#9663)
MINOR: Increase unit test coverage of method
ProcessorTopology#updateSourceTopics() (apache#9654)
  MINOR: fix reading SSH output in Streams system tests (apache#9665)
  KAFKA-10770: Remove duplicate defination of Metrics#getTags (apache#9659)
  KAFKA-10722: Described the types of the used state stores (apache#9607)
  KAFKA-10702; Skip bookkeeping of empty transactions (apache#9632)
  MINOR: Remove erroneous extra <code> in design doc (apache#9657)
KAFKA-10736 Convert transaction coordinator metadata schemas to use g…
(apache#9611)
  MINOR: Update vagrant/tests readme (apache#9650)
  KAFKA-10720: Document prohibition on header mutation by SMTs (apache#9597)
  KAFKA-10713: Stricter protocol parsing in hostnames (apache#9593)
  KAFKA-10565: Only print console producer prompt with a tty (apache#9644)
  MINOR: fix listeners doc to close <code> properly (apache#9655)
  MINOR: Remove unnecessary statement from WorkerConnector#doRun (apache#9653)
KAFKA-10758: ProcessorTopology should only consider its own nodes when
updating regex source topics (apache#9648)
KAFKA-10754: fix flaky tests by waiting kafka streams be in running
state before assert (apache#9629)
  MINOR: Upgrade to Scala 2.13.4 (apache#9643)
  MINOR: Update build and test dependencies (apache#9645)
MINOR: Remove redundant argument from TaskMetricsGroup#recordCommit
(apache#9642)
  ...

clients/src/main/java/org/apache/kafka/clients/ClientRequest.java
clients/src/main/java/org/apache/kafka/clients/NetworkClient.java
clients/src/main/java/org/apache/kafka/common/requests/AbstractRequest.java
clients/src/main/java/org/apache/kafka/common/requests/AbstractRequestResponse.java
clients/src/main/java/org/apache/kafka/common/requests/AbstractResponse.java
clients/src/main/java/org/apache/kafka/common/requests/AlterConfigsResponse.java
clients/src/main/java/org/apache/kafka/common/requests/ApiVersionsResponse.java
clients/src/main/java/org/apache/kafka/common/requests/FetchRequest.java
clients/src/main/java/org/apache/kafka/common/requests/FetchResponse.java
clients/src/main/java/org/apache/kafka/common/requests/LeaderAndIsrRequest.java
clients/src/main/java/org/apache/kafka/common/requests/ListOffsetRequest.java
clients/src/main/java/org/apache/kafka/common/requests/ListOffsetResponse.java
clients/src/main/java/org/apache/kafka/common/requests/OffsetsForLeaderEpochRequest.java
clients/src/main/java/org/apache/kafka/common/requests/OffsetsForLeaderEpochResponse.java
clients/src/main/java/org/apache/kafka/common/requests/ProduceRequest.java
clients/src/main/java/org/apache/kafka/common/requests/ProduceResponse.java
clients/src/main/java/org/apache/kafka/common/requests/RequestHeader.java
clients/src/main/java/org/apache/kafka/common/requests/StopReplicaRequest.java
clients/src/main/java/org/apache/kafka/common/requests/UpdateMetadataRequest.java
clients/src/test/java/org/apache/kafka/clients/NetworkClientTest.java
clients/src/test/java/org/apache/kafka/clients/consumer/internals/FetcherTest.java
clients/src/test/java/org/apache/kafka/clients/producer/internals/SenderTest.java
clients/src/test/java/org/apache/kafka/common/requests/ProduceResponseTest.java
clients/src/test/java/org/apache/kafka/common/requests/RequestContextTest.java
clients/src/test/java/org/apache/kafka/common/requests/RequestHeaderTest.java
clients/src/test/java/org/apache/kafka/common/security/authenticator/SaslAuthenticatorTest.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rondagostino rondagostino rondagostino left review comments

@chia7712 chia7712 chia7712 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ijuma @rondagostino @chia7712