[ 
https://issues.apache.org/jira/browse/GEODE-8435?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17180678#comment-17180678
 ] 

ASF GitHub Bot commented on GEODE-8435:
---------------------------------------

jinmeiliao commented on a change in pull request #5462:
URL: https://github.com/apache/geode/pull/5462#discussion_r473185831



##########
File path: 
geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/ConnectCommand.java
##########
@@ -197,6 +201,32 @@ public ResultModel connect(
     }
   }
 
+  /**
+   * because remote serialization version was not exposed until 1.12, but we 
are compatible back to
+   * 1.10, then any 1.x remote serialization version implies compatibility; 
otherwise make some
+   * narrow assumptions about product version numbers known to be associated 
with 1.10/1.11 to fill
+   * the gap.
+   *
+   * It seems reasonable to commit to future compatibility with any future 
Geode of the same major,
+   * but we should probably draw the line somewhere and not promise that gfsh 
1.x will be eternally
+   * compatible with Geode 2.x and later
+   */
+  static boolean isCompatible(String ourSerializationVersion, String 
remoteVersion,

Review comment:
       Maybe call this method shouldConnect?
   
   Your logic is correct, but maybe to make this more readable and have the 
code reflect our train of thought better, you can consider this logic flow:
   
   ```
   // client should connect to cluster version newer than 1.10
   boolean shouldConnect (String ourSerializationVersion, String remoteVersion, 
String remoteSerializationVersion) {
     if (remoteVersion == null) { // pre 1.5
        return false;
     }
     // post 1.12, this should always return true, since it's definitely after 
1.10.  I think this should fix the hydra test 
     // not sure if we need the ourSerilizationVersion to make this decision
     if (remoteSerializationVersion !=null) {
         return true;
     }
     // after 1.5 but before 1.12, use remoteVersion to deteremin if it's after 
1.10 or 9.9
     ......
   
   }
   ```




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[email protected]


> restore gfsh ability to connect if serialization versions match regardless of 
> product version
> ---------------------------------------------------------------------------------------------
>
>                 Key: GEODE-8435
>                 URL: https://issues.apache.org/jira/browse/GEODE-8435
>             Project: Geode
>          Issue Type: Bug
>          Components: gfsh
>            Reporter: Owen Nichols
>            Assignee: Owen Nichols
>            Priority: Major
>              Labels: pull-request-available
>
> This used to work but was broken recently by changes for GEODE-8331.  
> Although this does not impact Geode releases, it causes quite a headache for 
> developers that don't typically pass a product version in their gradle build 
> command.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to