Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add version check on client init #61

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

tellet-q
Copy link
Contributor

Send request to fetch server version during client init and compare with client version.

Introduce an optional checkCompatibility param (true by default). If it is true -> check versions during client init and generate a warning if versions are not compatible. Compatible versions are the ones that differ by one, i.e client version 1.6.x is compatible with server versions 1.5.x and 1.7.x but not 1.4.x or 1.8.x.

@tellet-q tellet-q requested a review from Anush008 December 20, 2024 12:35
Copy link
Member

@Anush008 Anush008 left a comment

Choose a reason for hiding this comment

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

👏

@tellet-q tellet-q requested a review from Anush008 December 20, 2024 14:44
Comment on lines +67 to +95
public static boolean isCompatible(String clientVersion, String serverVersion) {
if (clientVersion.isEmpty()) {
logger.warn("Unable to compare with client version {}", clientVersion);
return false;
}

if (serverVersion.isEmpty()) {
logger.warn("Unable to compare with server version {}", serverVersion);
return false;
}

if (clientVersion.equals(serverVersion)) {
return true;
}

try {
Version parsedServerVersion = parseVersion(serverVersion);
Version parsedClientVersion = parseVersion(clientVersion);

int majorDiff = Math.abs(parsedServerVersion.getMajor() - parsedClientVersion.getMajor());
if (majorDiff >= 1) {
return false;
}
return Math.abs(parsedServerVersion.getMinor() - parsedClientVersion.getMinor()) <= 1;
} catch (IllegalArgumentException e) {
logger.warn("Unable to compare versions: {}", e.getMessage());
return false;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public static boolean isCompatible(String clientVersion, String serverVersion) {
if (clientVersion.isEmpty()) {
logger.warn("Unable to compare with client version {}", clientVersion);
return false;
}
if (serverVersion.isEmpty()) {
logger.warn("Unable to compare with server version {}", serverVersion);
return false;
}
if (clientVersion.equals(serverVersion)) {
return true;
}
try {
Version parsedServerVersion = parseVersion(serverVersion);
Version parsedClientVersion = parseVersion(clientVersion);
int majorDiff = Math.abs(parsedServerVersion.getMajor() - parsedClientVersion.getMajor());
if (majorDiff >= 1) {
return false;
}
return Math.abs(parsedServerVersion.getMinor() - parsedClientVersion.getMinor()) <= 1;
} catch (IllegalArgumentException e) {
logger.warn("Unable to compare versions: {}", e.getMessage());
return false;
}
}
public static boolean isCompatible(String clientVersion, String serverVersion) {
try {
Version client = parseVersion(clientVersion);
Version server = parseVersion(serverVersion);
if (client.getMajor() != server.getMajor()) return false;
return Math.abs(client.getMinor() - server.getMinor()) <= 1;
} catch (IllegalArgumentException e) {
logger.warn("Version comparison failed: {}", e.getMessage());
return false;
}
}

Could be simpler like this I think.

Comment on lines +48 to +53
String rest =
parts.length > 2
? String.join(".", new ArrayList<>(Arrays.asList(parts).subList(2, parts.length)))
: "";

return new Version(major, minor, rest);
Copy link
Member

Choose a reason for hiding this comment

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

rest part of the version is not used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants