Replace FastAPI pure str comparison with SemVer comparison

[ayharano]

Description of the change

This is a solution proposal for #432: the current FastAPI version comparison fails for version 0.100.0-beta2 because the lexicographic comparison of the string value fails because "1" is less than "4" even if the numerical value 100 is higher than 41.

Before:

ipdb> self.min_version
'0.41.0'
ipdb> fastapi.__version__
'0.100.0-beta2'
ipdb> fastapi.__version__ < self.min_version  # due to lexicographic comparison, not proper SemVer comparison
True

After:

ipdb> self.min_version
'0.41.0'
ipdb> fastapi.__version__
'0.100.0-beta2'
ipdb> not is_current_version_higher_or_equal(fastapi.__version__, self.min_version)
False

Type of change

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Maintenance
• New release

Related issues

Shortcut stories and GitHub issues (delete irrelevant)

• Fix [SC-]
• Fix FastAPI version validation needs improvements #432

Checklists

Development

• Lint rules pass locally
• The code changed/added as part of this pull request has been covered with tests
• All tests related to the changed code pass in development

Code review

• This pull request has a descriptive title and information useful to a reviewer. There may be a screenshot or screencast attached
• "Ready for review" label attached to the PR and reviewers assigned
• Issue from task tracker has a link to this pull request
• Changes have been reviewed by at least one other engineer

[cforcey]

Outside perspective: I like how this is implemented with no new dependencies. I might have been tempted to lean on semver3, but understand this might create Python compatibility and other downstream complications. Will be interested to see how the discussion progresses.

[cforcey]

Checking in on any progress getting this merged? We are a little stuck on upgrades to fast-api while Rollbar misinterprets their 100 version as < 0.41+. FastAPI 0.41.0+ is required to use add_to() function

[danielmorell]

Hey @ayharano, thanks for the great work on this! Like @cforcey, I appreciate the fact that there are no 3rd party dependancies!

I added some unit tests based on the SemVer comparison on https://semver.org/#spec-item-11 with some additional versions to check FastAPI's non . separated pre-release identifiers (e.g. alpha2 not alpha.2).

Some of the tests are failing. I think it has to do with the - separator for the pre-release identifiers. I ran out of time today to dig into it. @ayharano is that something you can do?

[johnchristopherjones]

Per PEP 440, this could probably be simplified.

FastAPI's version needs to be PEP440 compliant, irrespective of SemVer's spec.

Unfortunately that means fully-complaint checking requires things like epoch prefixes and -dev sorting before -beta.

Fortunately however, I think we can outright ignore everything except the release segment (i.e., ignore -alphaN, -betaN, -devN, preN, etc.) for the purposes of compatibility checking. That is, unless we want to say the min version of FastAPI supported by Rollbar is something like 0.104.0-beta2 and 0.104.0-beta1 is not allowed. That seems very unlikely to be useful.

Suggested change

	def is_current_version_higher_or_equal(current_version, min_version):
	if current_version == min_version:
	return True
	for current_as_string, min_as_string in zip(
	current_version.split('.'),
	min_version.split('.'),
	):
	if current_as_string == min_as_string:
	continue
	try:
	current_as_int = int(current_as_string)
	except ValueError:
	current_as_int = None
	try:
	min_as_int = int(min_as_string)
	except ValueError:
	min_as_int = None
	if current_as_int is None or min_as_int is None:
	# If one of the parts fails the int conversion, compare as string
	return current_as_string > min_as_string
	else:
	if current_as_int == min_as_int:
	continue
	return current_as_int > min_as_int
	# Somehow the comparison didn't properly finish - defaulting to False
	return False
	def is_current_version_higher_or_equal(current_version, min_version):
	# We can ignore everything except the release segment
	current = tuple(map(int, re.findall(r"(?:^|[.])([0-9]+)", current_version)))
	minimum = tuple(map(int, re.findall(r"(?:^|[.])([0-9]+)", min_version)))
	return current >= minimum

[danielmorell]

@johnchristopherjones I agree that only trying to compare the release segment seems reasonable.

However, that regex also captures pre-release and dev segments if they include a .[0-9]. This does mean that 1.0.0-alpha.1 would become the tuple (1, 0, 0, 1). I am not great at regex. So, after a few failed attempts to solve this, I resorted to solving it in Python.