sanjeet006py commented on code in PR #2584:
URL: https://github.com/apache/phoenix/pull/2584#discussion_r3644920383
##########
phoenix-core/src/it/java/org/apache/phoenix/end2end/BackwardCompatibilityTestUtil.java:
##########
@@ -139,14 +139,18 @@ public static List<ClientVersionGroup>
computeClientVersionGroups() throws Excep
BackwardCompatibilityIT.class.getClassLoader().getResourceAsStream(COMPATIBLE_CLIENTS_JSON))
{
assertNotNull(inputStream);
JsonNode jsonNode = mapper.readTree(inputStream);
- JsonNode HBaseProfile = jsonNode.get(hbaseProfile);
- List<MavenCoordinates> artifacts = new ArrayList<>();
- for (final JsonNode clientVersion : HBaseProfile) {
- artifacts.add(mapper.treeToValue(clientVersion,
MavenCoordinates.class));
+ JsonNode hbaseProfileNode = jsonNode.get(hbaseProfile);
+ if (hbaseProfileNode == null) {
+ return clientVersionGroups;
}
- if (!artifacts.isEmpty()) {
- MavenCoordinates primary = artifacts.get(0);
- clientVersionGroups.add(new ClientVersionGroup(primary, artifacts));
+ for (final JsonNode groupNode : hbaseProfileNode) {
Review Comment:
This node represents a group of artifacts associated to a client version
i.e. `clientVersionGroupNode`. How about changing the name of variable to
reflect the same? Please correct me if I am understanding it wrong.
##########
phoenix-core/src/it/java/org/apache/phoenix/end2end/BackwardCompatibilityTestUtil.java:
##########
@@ -139,14 +139,18 @@ public static List<ClientVersionGroup>
computeClientVersionGroups() throws Excep
BackwardCompatibilityIT.class.getClassLoader().getResourceAsStream(COMPATIBLE_CLIENTS_JSON))
{
assertNotNull(inputStream);
JsonNode jsonNode = mapper.readTree(inputStream);
- JsonNode HBaseProfile = jsonNode.get(hbaseProfile);
- List<MavenCoordinates> artifacts = new ArrayList<>();
- for (final JsonNode clientVersion : HBaseProfile) {
- artifacts.add(mapper.treeToValue(clientVersion,
MavenCoordinates.class));
+ JsonNode hbaseProfileNode = jsonNode.get(hbaseProfile);
+ if (hbaseProfileNode == null) {
+ return clientVersionGroups;
}
- if (!artifacts.isEmpty()) {
- MavenCoordinates primary = artifacts.get(0);
- clientVersionGroups.add(new ClientVersionGroup(primary, artifacts));
+ for (final JsonNode groupNode : hbaseProfileNode) {
+ List<MavenCoordinates> artifacts = new ArrayList<>();
+ for (final JsonNode clientVersion : groupNode) {
Review Comment:
This node represents a single artifact within group of artifacts for a
specific HBase profile i.e. `artifactNode`. Same suggestion as above, how about
changing name of variable to reflect this logical mapping better?
##########
phoenix-core/src/it/resources/compatible_client_versions.json:
##########
@@ -1,50 +1,149 @@
{
"_comment": "Lists all phoenix compatible client versions against the
current branch version for a given hbase profile.",
"2.4": [
Review Comment:
@virajjasani shall we remove entry for 2.4 profile. As @ishitadixit-dot
highlighted master branch doesn't support HBase 2.4 anymore.
--
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]
