Skip to content

Conversation

@isapego
Copy link
Contributor

@isapego isapego commented Jan 23, 2026

https://issues.apache.org/jira/browse/IGNITE-27645

Currently, for any version like 3.2.0-beta1, 3.2.0-beta2, 3.2.0 we will generate the same version for the python DB API package - 3.2.0. This won't do as we can not upload multiple packages with the same version. So this patch addresses it and generates appropriate versions like 3.2.0b1 or 3.2.0b2 in cases like this.

See https://packaging.python.org/en/latest/discussions/versioning/ for details

Also removed unused code for wheels generation using Docker.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes Python version generation for the DB API by implementing proper conversion to Python's version format (PEP 440) and removes unused Docker-based Gradle build tasks for wheel creation.

Changes:

  • Adds pythonProjectVersion() function to convert project versions to Python-compatible format (e.g., "3.2.0-alpha1" → "3.2.0a1")
  • Replaces simpleProjectVersion() with pythonProjectVersion() in updatePythonVersion() to preserve pre-release identifiers
  • Removes Docker plugin dependency and all Docker-related imports, variables, and tasks (manylinux wheels, Windows wheels, source distribution tasks)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +271 to +279
def pythonAddition = addition
.toLowerCase()
.replace("alpha", "a")
.replace("beta", "b")
.replace("patch", "p")

if (addition.startsWith("alpha")
|| addition.startsWith("beta")
|| addition.startsWith("patch"))
Copy link

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
def pythonAddition = addition
.toLowerCase()
.replace("alpha", "a")
.replace("beta", "b")
.replace("patch", "p")
if (addition.startsWith("alpha")
|| addition.startsWith("beta")
|| addition.startsWith("patch"))
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"))

Copilot uses AI. Check for mistakes.
* See https://packaging.python.org/en/latest/discussions/versioning/
*/
private String pythonProjectVersion(String version) {
def (simple, addition) = version.tokenize('-')
Copy link

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
def (simple, addition) = version.tokenize('-')
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)
}

Copilot uses AI. Check for mistakes.
Comment on lines +279 to +280
|| addition.startsWith("patch"))
{
Copy link

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
|| addition.startsWith("patch"))
{
|| addition.startsWith("patch")) {

Copilot uses AI. Check for mistakes.
Comment on lines +265 to +284
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
}
Copy link

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
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.

1 participant