cmccabe commented on code in PR #12513:
URL: https://github.com/apache/kafka/pull/12513#discussion_r949693026
##########
metadata/src/test/java/org/apache/kafka/controller/FeatureControlManagerTest.java:
##########
@@ -222,52 +227,130 @@ public void testFeatureControlIterator() throws
Exception {
manager.iterator(Long.MAX_VALUE));
}
+ private static final FeatureControlManager.Builder TEST_MANAGER_BUILDER1 =
+ new FeatureControlManager.Builder().
+ setQuorumFeatures(features(MetadataVersion.FEATURE_NAME,
+ MetadataVersion.IBP_3_3_IV0.featureLevel(),
MetadataVersion.IBP_3_3_IV3.featureLevel())).
+ setMetadataVersion(MetadataVersion.IBP_3_3_IV2);
+
@Test
public void testApplyMetadataVersionChangeRecord() {
- QuorumFeatures features = features(MetadataVersion.FEATURE_NAME,
- MetadataVersion.IBP_3_0_IV1.featureLevel(),
MetadataVersion.IBP_3_3_IV0.featureLevel());
- FeatureControlManager manager = new FeatureControlManager.Builder().
- setQuorumFeatures(features).build();
+ FeatureControlManager manager = TEST_MANAGER_BUILDER1.build();
manager.replay(new FeatureLevelRecord().
setName(MetadataVersion.FEATURE_NAME).
- setFeatureLevel(MetadataVersion.IBP_3_0_IV1.featureLevel()));
- assertEquals(MetadataVersion.IBP_3_0_IV1, manager.metadataVersion());
+ setFeatureLevel(MetadataVersion.IBP_3_3_IV3.featureLevel()));
+ assertEquals(MetadataVersion.IBP_3_3_IV3, manager.metadataVersion());
}
@Test
- public void testDowngradeMetadataVersion() {
- QuorumFeatures features = features(MetadataVersion.FEATURE_NAME,
- MetadataVersion.IBP_3_2_IV0.featureLevel(),
MetadataVersion.IBP_3_3_IV0.featureLevel());
- FeatureControlManager manager = new FeatureControlManager.Builder().
- setQuorumFeatures(features).
- setMetadataVersion(MetadataVersion.IBP_3_3_IV0).
- build();
- assertEquals(manager.metadataVersion(), MetadataVersion.IBP_3_3_IV0);
+ public void
testCannotDowngradeToVersionBeforeMinimumSupportedKraftVersion() {
+ FeatureControlManager manager = TEST_MANAGER_BUILDER1.build();
+ assertEquals(ControllerResult.of(Collections.emptyList(),
+ singletonMap(MetadataVersion.FEATURE_NAME, new
ApiError(Errors.INVALID_UPDATE_VERSION,
Review Comment:
Hmm... I have a different view about INVALID_REQUEST. I thought that error
code was supposed to be a catch-all for requests that were malformed. I don't
think we should be using it in APIs to indicate that the user messed up. In
theory a well-programmed client should not send invalid requests.
I'm used to the way errno works in C, where there's a small number of error
codes and they're heavily overloaded. For example, a bunch of APIs return
ENOENT if the entity they're looking for doesn't exist, or EEXIST if it does
and that's a problem.
I sort of look askance at Kafka's approach. Like did we really need a
separate error code for DELEGATION_TOKEN_NOT_FOUND, GROUP_ID_NOT_FOUND, and
RESOURCE_NOT_FOUND? Or could we just have a NOT_FOUND error code and return it
where appropriate. It's not like you are going to get back
DELEGATION_TOKEN_NOT_FOUND from an API that deals with groups, so what's the
issue? You are not adding more descriptive power by having an X_NOT_FOUND,
Y_NOT_FOUND, Z_NOT_FOUND If only one of them can be used in any given scenario.
--
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]
