BAI-1299 Python semantic versioning bugfix #1305
Conversation
bw27492
commented
Jun 3, 2024
bw27492
commented
- Semantic versons containing 'v' will no longer break client
| @version.setter | ||
| def version(self, value): | ||
| if isinstance(value, str): | ||
| version_obj = value.replace("v", "") |
There was a problem hiding this comment.
This replaces all v characters throughout the string. Which could be within the build or the pre-release. Instead use
version_obj = value.removeprefix("v")There was a problem hiding this comment.
Update - this was added in 3.9 so fails our Python 3.8 tests :( have changed to an older method
| raise TypeError("Provided version not of a supported type.") | ||
|
|
||
| self._version_obj = version_obj | ||
| self._version_raw = value |
There was a problem hiding this comment.
Here it's probably better to fetch from the object rather than the value. In this method coerce is used however this is not used on the backend validation. so will fail with strings like '0.1.2.3.4'
There was a problem hiding this comment.
The _version_raw attribute is the same value used to get/create releases on the server - so if the user inputs an incorrect value the server will return an error anyway. Also if we get the semver from the object, the v will have been removed so if the user calls .update() the server won't find the right release (e.g. the client will try to get 2.0.0 as opposed to v2.0.0)
| return self._version_obj | ||
|
|
||
| @version.setter | ||
| def version(self, value): |
There was a problem hiding this comment.
I would put this validation in post_release as well within the create classmethod just to prevent an unnecessary response when creating that endpoint.
There was a problem hiding this comment.
I don't think we want to fully stop users from adding the 'v' using the post_release method, as the front and backend will allow this. It only presented an issue when creating the Version() object which is used to compare Release() objects etc, which is resolved with this PR whilst allowing the user to include the 'v' upon creation if they want to
| @version.setter | ||
| def version(self, value): |
There was a problem hiding this comment.
I think it's best to make this private. Just to avoid any problematic updating.
There was a problem hiding this comment.
Done - alternative fix for this. If the user tries to set the version after it has already been defined, it will throw a BailoException. Making the attribute itself private caused larger issues in the code base and created a breaking change
