chia7712 commented on code in PR #16648:
URL: https://github.com/apache/kafka/pull/16648#discussion_r1730562364
##########
clients/src/main/java/org/apache/kafka/clients/admin/ExpireDelegationTokenOptions.java:
##########
@@ -25,10 +25,19 @@
* The API of this class is evolving, see {@link Admin} for details.
*/
@InterfaceStability.Evolving
-public class ExpireDelegationTokenOptions extends
AbstractOptions<ExpireDelegationTokenOptions> {
+public class ExpireDelegationTokenOptions
Review Comment:
Could you please revert this unrelated change?
##########
clients/src/main/java/org/apache/kafka/clients/admin/ExpireDelegationTokenOptions.java:
##########
@@ -25,10 +25,19 @@
* The API of this class is evolving, see {@link Admin} for details.
*/
@InterfaceStability.Evolving
-public class ExpireDelegationTokenOptions extends
AbstractOptions<ExpireDelegationTokenOptions> {
+public class ExpireDelegationTokenOptions
+ extends AbstractOptions<ExpireDelegationTokenOptions> {
+
private long expiryTimePeriodMs = -1L;
- public ExpireDelegationTokenOptions expiryTimePeriodMs(long
expiryTimePeriodMs) {
+ /**
+ * @param expiryTimePeriodMs the time period until we should expire this
token.
+ * {@code expiryTimePeriodMs} >= 0: the token will update the `expiration
timestamp` if the current expiration timestamp is small than (now +
expiryTimePeriodMs).
Review Comment:
Sorry that could you please revise it to "{@code expiryTimePeriodMs} >= 0:
the token will update the `expiration timestamp` to min(now +
expiryTimePeriodMs, maxTimestamp)`
##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -106,19 +150,22 @@ class PlaintextAdminIntegrationTest extends
BaseAdminIntegrationTest {
config.put(AdminClientConfig.CLIENT_ID_CONFIG, clientId)
client = Admin.create(config)
- val entity = new ClientQuotaEntity(Map(ClientQuotaEntity.CLIENT_ID ->
clientId).asJava)
- val configEntries =
Map(QuotaConfigs.CONTROLLER_MUTATION_RATE_OVERRIDE_CONFIG -> 1.0,
QuotaConfigs.CONSUMER_BYTE_RATE_OVERRIDE_CONFIG -> 3.0)
- client.alterClientQuotas(Seq(new ClientQuotaAlteration(entity,
configEntries.map {case (k, v) =>
- new
ClientQuotaAlteration.Op(k,v)}.asJavaCollection)).asJavaCollection).all.get
+ try {
+ val entity = new ClientQuotaEntity(Map(ClientQuotaEntity.CLIENT_ID ->
clientId).asJava)
+ val configEntries =
Map(QuotaConfigs.CONTROLLER_MUTATION_RATE_OVERRIDE_CONFIG -> 1.0,
QuotaConfigs.CONSUMER_BYTE_RATE_OVERRIDE_CONFIG -> 3.0)
+ client.alterClientQuotas(Seq(new ClientQuotaAlteration(entity,
configEntries.map { case (k, v) =>
+ new ClientQuotaAlteration.Op(k, v)
+ }.asJavaCollection)).asJavaCollection).all.get
- TestUtils.waitUntilTrue(() => {
- // wait for our ClientQuotaEntity to be set
-
client.describeClientQuotas(ClientQuotaFilter.all()).entities().get().size == 1
- }, "Timed out waiting for quota config to be propagated to all servers")
+ TestUtils.waitUntilTrue(() => {
+ // wait for our ClientQuotaEntity to be set
+
client.describeClientQuotas(ClientQuotaFilter.all()).entities().get().size == 1
+ }, "Timed out waiting for quota config to be propagated to all servers")
- val quotaEntities =
client.describeClientQuotas(ClientQuotaFilter.all()).entities().get()
+ val quotaEntities =
client.describeClientQuotas(ClientQuotaFilter.all()).entities().get()
- assertEquals(configEntries,quotaEntities.get(entity).asScala)
+ assertEquals(configEntries, quotaEntities.get(entity).asScala)
+ } finally client.close(time.Duration.ZERO)
Review Comment:
this client is created normally (correct port), so it can be closed as
usual. that means we don't need to add try-finally, as it will be closed by the
`@AfterEach`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]