-
Notifications
You must be signed in to change notification settings - Fork 571
CASSPYTHON-10 Update cassandra.util.Version to better support Cassandra version strings #1273
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
base: trunk
Are you sure you want to change the base?
Changes from all commits
972a262
9195f76
99a95ed
d63ec96
21c19ac
a11ee11
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1692,57 +1692,50 @@ def __repr__(self): | |
| self.lower_bound, self.upper_bound, self.value | ||
| ) | ||
|
|
||
| VERSION_REGEX = re.compile("^(\\d+)\\.(\\d+)(\\.\\d+)?(\\.\\d+)?([~\\-]\\w[.\\w]*(?:-\\w[.\\w]*)*)?(\\+[.\\w]+)?$") | ||
|
|
||
| @total_ordering | ||
| class Version(object): | ||
| """ | ||
| Internal minimalist class to compare versions. | ||
| A valid version is: <int>.<int>.<int>.<int or str>. | ||
|
|
||
| TODO: when python2 support is removed, use packaging.version. | ||
| Representation of a Cassandra version. Mostly follows the implementation of the same logic in the Java driver; | ||
| see https://github.com/apache/cassandra-java-driver/blob/4.19.2/core/src/main/java/com/datastax/oss/driver/api/core/Version.java | ||
| """ | ||
|
|
||
| _version = None | ||
| major = None | ||
| minor = 0 | ||
| patch = 0 | ||
| build = 0 | ||
| prerelease = 0 | ||
|
|
||
| def __init__(self, version): | ||
| self._version = version | ||
| if '-' in version: | ||
| version_without_prerelease, self.prerelease = version.split('-', 1) | ||
| else: | ||
| version_without_prerelease = version | ||
| parts = list(reversed(version_without_prerelease.split('.'))) | ||
| if len(parts) > 4: | ||
| prerelease_string = "-{}".format(self.prerelease) if self.prerelease else "" | ||
| log.warning("Unrecognized version: {}. Only 4 components plus prerelease are supported. " | ||
| "Assuming version as {}{}".format(version, '.'.join(parts[:-5:-1]), prerelease_string)) | ||
|
|
||
| match = VERSION_REGEX.match(version) | ||
| if not match: | ||
| raise ValueError("Version string {0} did not match expected format".format(version)) | ||
|
|
||
| self.major = int(match[1]) | ||
| self.minor = int(match[2]) | ||
|
|
||
| try: | ||
| self.major = int(parts.pop()) | ||
| except ValueError as e: | ||
| raise ValueError( | ||
| "Couldn't parse version {}. Version should start with a number".format(version))\ | ||
| .with_traceback(e.__traceback__) | ||
| self.patch = self._cleanup_int(match[3]) | ||
| except: | ||
| self.patch = 0 | ||
|
|
||
| try: | ||
| self.minor = int(parts.pop()) if parts else 0 | ||
| self.patch = int(parts.pop()) if parts else 0 | ||
| self.build = self._cleanup_int(match[4]) | ||
| except: | ||
| self.build = 0 | ||
|
|
||
| if parts: # we have a build version | ||
| build = parts.pop() | ||
| try: | ||
| self.build = int(build) | ||
| except ValueError: | ||
| self.build = build | ||
| except ValueError: | ||
| assumed_version = "{}.{}.{}.{}-{}".format(self.major, self.minor, self.patch, self.build, self.prerelease) | ||
| log.warning("Unrecognized version {}. Assuming version as {}".format(version, assumed_version)) | ||
| try: | ||
| self.prerelease = self._cleanup_str(match[5]) | ||
| except: | ||
| self.prerelease = 0 | ||
|
Comment on lines
1714
to
+1727
|
||
|
|
||
| # Trim off the leading '.' characters and convert the discovered value to an integer | ||
| def _cleanup_int(self, instr): | ||
| return int(instr[1:]) if instr else 0 | ||
|
|
||
| # Trim off the leading '.' or '~' characters and just return the string directly | ||
| def _cleanup_str(self, instr): | ||
| return instr[1:] if instr else 0 | ||
|
|
||
| def __hash__(self): | ||
| return self._version | ||
| return hash((self.major, self.minor, self.patch, self.build, self.prerelease)) | ||
|
|
||
| def __repr__(self): | ||
| version_string = "Version({0}, {1}, {2}".format(self.major, self.minor, self.patch) | ||
|
|
@@ -1757,48 +1750,32 @@ def __repr__(self): | |
| def __str__(self): | ||
| return self._version | ||
|
|
||
| @staticmethod | ||
| def _compare_version_part(version, other_version, cmp): | ||
| if not (isinstance(version, int) and | ||
| isinstance(other_version, int)): | ||
| version = str(version) | ||
| other_version = str(other_version) | ||
|
|
||
| return cmp(version, other_version) | ||
|
|
||
| def __eq__(self, other): | ||
| if not isinstance(other, Version): | ||
| return NotImplemented | ||
|
|
||
| return (self.major == other.major and | ||
| self.minor == other.minor and | ||
| self.patch == other.patch and | ||
| self._compare_version_part(self.build, other.build, lambda s, o: s == o) and | ||
| self._compare_version_part(self.prerelease, other.prerelease, lambda s, o: s == o) | ||
| self.build == other.build and | ||
| self.prerelease == other.prerelease | ||
| ) | ||
|
|
||
| def __gt__(self, other): | ||
| if not isinstance(other, Version): | ||
| return NotImplemented | ||
|
|
||
| is_major_ge = self.major >= other.major | ||
| is_minor_ge = self.minor >= other.minor | ||
| is_patch_ge = self.patch >= other.patch | ||
| is_build_gt = self._compare_version_part(self.build, other.build, lambda s, o: s > o) | ||
| is_build_ge = self._compare_version_part(self.build, other.build, lambda s, o: s >= o) | ||
|
|
||
| # By definition, a prerelease comes BEFORE the actual release, so if a version | ||
| # doesn't have a prerelease, it's automatically greater than anything that does | ||
| if self.prerelease and not other.prerelease: | ||
| is_prerelease_gt = False | ||
| if self.major != other.major: | ||
| return self.major > other.major | ||
| elif self.minor != other.minor: | ||
| return self.minor > other.minor | ||
| elif self.patch != other.patch: | ||
| return self.patch > other.patch | ||
| elif self.build != other.build: | ||
| return self.build > other.build | ||
| elif self.prerelease and not other.prerelease: | ||
| return False | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Python tuples can be compared directly, so something like this should work
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I honestly didn't realize that would work @bschoening, thanks for mentioning that! After looking at it for a bit, though, I'm kind of inclined to leave it as a sequence if/else strings. My only rationale is that it reads much more clearly to a casual reader. That might be a personal bias on my part because the tuple comparison syntax you reference was unfamiliar to me (and I'm open to reconsideration if that's the case) but the current version immediately conveys the intent behind the code on even a surface reading... which is probably beneficial. |
||
| elif other.prerelease and not self.prerelease: | ||
| is_prerelease_gt = True | ||
| return True | ||
| else: | ||
| is_prerelease_gt = self._compare_version_part(self.prerelease, other.prerelease, lambda s, o: s > o) \ | ||
|
|
||
| return (self.major > other.major or | ||
| (is_major_ge and self.minor > other.minor) or | ||
| (is_major_ge and is_minor_ge and self.patch > other.patch) or | ||
| (is_major_ge and is_minor_ge and is_patch_ge and is_build_gt) or | ||
| (is_major_ge and is_minor_ge and is_patch_ge and is_build_ge and is_prerelease_gt) | ||
| ) | ||
| return self.prerelease > other.prerelease | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new
Versionimplementation removed the class docstring entirely. Sincecassandra.util.Versionis used outside this module (e.g. incassandra.metadataand unit tests), it should keep an up-to-date docstring describing the supported version formats and comparison semantics.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a fair point; I'll add an updated doc string back in.