You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "isapego (via GitHub)" <gi...@apache.org> on 2026/01/23 01:25:44 UTC
[PR] IGNITE-27645 Python DB API: Fix version generation [ignite-3]
isapego opened a new pull request, #7466:
URL: https://github.com/apache/ignite-3/pull/7466
https://issues.apache.org/jira/browse/IGNITE-27645
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-27645 Python DB API: Fix version generation [ignite-3]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #7466:
URL: https://github.com/apache/ignite-3/pull/7466#discussion_r2719258520
##########
modules/platforms/build.gradle:
##########
@@ -267,6 +258,31 @@ private String simpleProjectVersion(String version) {
return version
}
+/**
+ * Converts the version to the python version format.
+ * See https://packaging.python.org/en/latest/discussions/versioning/
+ */
+private String pythonProjectVersion(String version) {
+ def (simple, addition) = version.tokenize('-')
+ if (addition == null) {
+ return simple
+ }
+
+ def pythonAddition = addition
+ .toLowerCase()
+ .replace("alpha", "a")
+ .replace("beta", "b")
+ .replace("patch", "p")
+
+ if (addition.startsWith("alpha")
+ || addition.startsWith("beta")
+ || addition.startsWith("patch"))
Review Comment:
The conditional checks use the original `addition` variable instead of the lowercased `pythonAddition` variable. This means the checks will fail for uppercase pre-release identifiers (e.g., "ALPHA", "BETA", "PATCH"). The condition should check `pythonAddition.startsWith()` instead of `addition.startsWith()` since the replacements were made on the lowercased version.
```suggestion
def lowercasedAddition = addition.toLowerCase()
def pythonAddition = lowercasedAddition
.replace("alpha", "a")
.replace("beta", "b")
.replace("patch", "p")
if (lowercasedAddition.startsWith("alpha")
|| lowercasedAddition.startsWith("beta")
|| lowercasedAddition.startsWith("patch"))
```
##########
modules/platforms/build.gradle:
##########
@@ -267,6 +258,31 @@ private String simpleProjectVersion(String version) {
return version
}
+/**
+ * Converts the version to the python version format.
+ * See https://packaging.python.org/en/latest/discussions/versioning/
+ */
+private String pythonProjectVersion(String version) {
+ def (simple, addition) = version.tokenize('-')
+ if (addition == null) {
+ return simple
+ }
+
+ def pythonAddition = addition
+ .toLowerCase()
+ .replace("alpha", "a")
+ .replace("beta", "b")
+ .replace("patch", "p")
+
+ if (addition.startsWith("alpha")
+ || addition.startsWith("beta")
+ || addition.startsWith("patch"))
+ {
Review Comment:
The opening brace is placed on its own line, which is inconsistent with the style used elsewhere in this file where braces are typically on the same line as the conditional statement. For consistency, consider moving the opening brace to the same line as the last condition or the first condition.
```suggestion
|| addition.startsWith("patch")) {
```
##########
modules/platforms/build.gradle:
##########
@@ -267,6 +258,31 @@ private String simpleProjectVersion(String version) {
return version
}
+/**
+ * Converts the version to the python version format.
+ * See https://packaging.python.org/en/latest/discussions/versioning/
+ */
+private String pythonProjectVersion(String version) {
+ def (simple, addition) = version.tokenize('-')
Review Comment:
The `tokenize('-')` method splits on all occurrences of '-', not just the first one. If a version string contains multiple dashes (e.g., "3.2.0-alpha-1"), the destructuring assignment will fail because it expects exactly 2 elements but will receive more. Consider using `split('-', 2)` or handling the first dash only with `indexOf('-')` to safely handle multi-dash version strings.
```suggestion
def dashIndex = version.indexOf('-')
def simple
def addition
if (dashIndex == -1) {
simple = version
addition = null
} else {
simple = version.substring(0, dashIndex)
addition = version.substring(dashIndex + 1)
}
```
##########
modules/platforms/build.gradle:
##########
@@ -267,6 +258,31 @@ private String simpleProjectVersion(String version) {
return version
}
+/**
+ * Converts the version to the python version format.
+ * See https://packaging.python.org/en/latest/discussions/versioning/
+ */
+private String pythonProjectVersion(String version) {
+ def (simple, addition) = version.tokenize('-')
+ if (addition == null) {
+ return simple
+ }
+
+ def pythonAddition = addition
+ .toLowerCase()
+ .replace("alpha", "a")
+ .replace("beta", "b")
+ .replace("patch", "p")
+
+ if (addition.startsWith("alpha")
+ || addition.startsWith("beta")
+ || addition.startsWith("patch"))
+ {
+ return simple + pythonAddition
+ }
+ return simple + '.' + pythonAddition
+}
Review Comment:
The version conversion logic does not handle release candidate versions properly. According to PEP 440, release candidates should be formatted as "3.2.0rc1" (without a dot separator), similar to alpha and beta. However, versions like "3.2.0-rc1" will be converted to "3.2.0.rc1" (with a dot), which is not the standard format. Consider adding support for "rc" by including it in the replace logic and the startsWith check.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-27645 Python DB API: Fix version generation [ignite-3]
Posted by "isapego (via GitHub)" <gi...@apache.org>.
isapego merged PR #7466:
URL: https://github.com/apache/ignite-3/pull/7466
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org