Skip to content

Gradle: python code format/check #1986

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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

MonkeyCanCode
Copy link
Contributor

Here is a sample PR for enable python code format/check from conclusion derived from #1954 (as it is a sample PR, I didn't update the doc as I would like to get more feedback from community first).

Now with this approach, I am using use-python module from gradle to setup ruff which later get uses for code check/format/auto-fix etc. Then based off ruff, I introduced two tasks within gradle for code check and code format.

Here is the sample output from the two commands mentioned above that are newly added:

➜  polaris git:(gradle_python) ./gradlew ruffCheck
Configuration on demand is an incubating feature.

> Task :checkPython
Using python 3.9.6 from /Users/yong/Desktop/GitHome/polaris/.gradle/python (.gradle/python/bin/python)
Using pip 21.2.4 from /Users/yong/Desktop/GitHome/polaris/.gradle/python/lib/python3.9/site-packages/pip (python 3.9)

> Task :pipInstall
All required modules are already installed with correct versions
[python] .gradle/python/bin/python -m pip list --format=columns
         Package    Version
         ---------- -------
         pip        21.2.4
         ruff       0.12.1
         setuptools 58.0.4
         WARNING: You are using pip version 21.2.4; however, version 25.1.1 is available.
         You should consider upgrading via the '/Users/yong/Desktop/GitHome/polaris/.gradle/python/bin/python -m pip install --upgrade pip' command.

> Task :ruffCheck
All checks passed!

[Incubating] Problems report is available at: file:///Users/yong/Desktop/GitHome/polaris/build/reports/problems/problems-report.html

Deprecated Gradle features were used in this build, making it incompatible with Gradle 9.0.

You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.

For more on this, please refer to https://docs.gradle.org/8.14.2/userguide/command_line_interface.html#sec:command_line_warnings in the Gradle documentation.

BUILD SUCCESSFUL in 2s
12 actionable tasks: 3 executed, 9 up-to-date
➜  polaris git:(gradle_python) ./gradlew ruffFormat
Configuration on demand is an incubating feature.

> Task :checkPython
Using python 3.9.6 from /Users/yong/Desktop/GitHome/polaris/.gradle/python (.gradle/python/bin/python)
Using pip 21.2.4 from /Users/yong/Desktop/GitHome/polaris/.gradle/python/lib/python3.9/site-packages/pip (python 3.9)

> Task :pipInstall
All required modules are already installed with correct versions
[python] .gradle/python/bin/python -m pip list --format=columns
         Package    Version
         ---------- -------
         pip        21.2.4
         ruff       0.12.1
         setuptools 58.0.4
         WARNING: You are using pip version 21.2.4; however, version 25.1.1 is available.
         You should consider upgrading via the '/Users/yong/Desktop/GitHome/polaris/.gradle/python/bin/python -m pip install --upgrade pip' command.

> Task :ruffFormat
17 files left unchanged

[Incubating] Problems report is available at: file:///Users/yong/Desktop/GitHome/polaris/build/reports/problems/problems-report.html

Deprecated Gradle features were used in this build, making it incompatible with Gradle 9.0.

You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.

For more on this, please refer to https://docs.gradle.org/8.14.2/userguide/command_line_interface.html#sec:command_line_warnings in the Gradle documentation.

BUILD SUCCESSFUL in 1s
12 actionable tasks: 3 executed, 9 up-to-date
➜  polaris git:(gradle_python)

Here are the concerns/questions on my end:

  1. there doesn't appear to be a well maintain python module for gradle (pygradle from LinkedIn has no new release since 2020 and the one I am using only have 6 developers and last release is end of 2024)
  2. from the Makefile created for python module (https://github.com/apache/polaris/blob/main/client/python/Makefile#L34), there appear to be some effort went in around python code base. Does it makes more sense to move away from gradle for python code base and use Makefile route instead?

@eric-maynard @HonahX

@HonahX
Copy link
Contributor

HonahX commented Jul 3, 2025

Thanks for the example PR! Sorry I missed the discussion earlier, just want to understand more about the motivation behind this. Do we want to add python format check to gradle because we want format issue stops CI before enterring the real test cases?

Currently the python workflow can do that via the lint step, which is before the unit and integration tests

- name: Lint
working-directory: client/python
run: |
make lint

IMHO, the python client should be isolated from the main java project and all the dependency management/test/checking should happen with in that folder via poetry and some other python specific tools. And format failure or test failure should not cause irrelevant CI workflow to fail (e.g. Java Gradle test). This could help make it simpler to maintain python and outer java project.

We do have open api client generation configured in gradle now for existing quickstart and regtest, but I hope we could make that automated during package installation: #1885

But please let me know if I miss anything or if there are other good thing about gradle python format check : )

@eric-maynard
Copy link
Contributor

@HonahX

Do we want to add python format check to gradle because we want format issue stops CI before enterring the real test cases?

I'd like to have the same setup as the rest of the code -- CI fails if you have a format issue, and ./gradlew format fixes it.

IMHO, the python client should be isolated from the main java project and all the dependency management/test/checking should happen with in that folder via poetry and some other python specific tools. And format failure or test failure should not cause irrelevant CI workflow to fail (e.g. Java Gradle test)

It makes sense to use python-specific tooling (ruff?) and to have python-specific commands... but the gradle task checkstyleMain isn't called checkStyleJava.

We already check the python code in the gradle task copiedCodeChecks and as you pointed out we also have the (short-lived?) gradle task regeneratePythonClient. Having a gradle task to check/fix style issues seems convenient and not a huge leap from what we already have.

Agreed that irrelevant CI shouldn't fail -- but there actually is no CI action called Jave Gradle test... Regression Tests is failing, for example, but that's not Java specific and in fact runs Python code today. The Python CI should ideally fail if there are Python style issues.

@MonkeyCanCode
Copy link
Contributor Author

MonkeyCanCode commented Jul 3, 2025

@HonahX

Do we want to add python format check to gradle because we want format issue stops CI before enterring the real test cases?

I'd like to have the same setup as the rest of the code -- CI fails if you have a format issue, and ./gradlew format fixes it.

IMHO, the python client should be isolated from the main java project and all the dependency management/test/checking should happen with in that folder via poetry and some other python specific tools. And format failure or test failure should not cause irrelevant CI workflow to fail (e.g. Java Gradle test)

It makes sense to use python-specific tooling (ruff?) and to have python-specific commands... but the gradle task checkstyleMain isn't called checkStyleJava.

We already check the python code in the gradle task copiedCodeChecks and as you pointed out we also have the (short-lived?) gradle task regeneratePythonClient. Having a gradle task to check/fix style issues seems convenient and not a huge leap from what we already have.

Agreed that irrelevant CI shouldn't fail -- but there actually is no CI action called Jave Gradle test... Regression Tests is failing, for example, but that's not Java specific and in fact runs Python code today. The Python CI should ideally fail if there are Python style issues.

@HonahX a bit more context on this PR:

While i was checking for what to pick up next, I was not aware we have a Makefile that does this already and noticed the code has a bit free-style. Thus, I ended up ran a manual ruff locally to format the code then check-in the changes (#1954). As part of this PR, @eric-maynard suggests we should automate this as part of check-in as how we are doing today such as spotless apply for formatting java code. Thus, I began explore what options are out there with gradle. It turns out there is no a popular plugin for gradle to work with python (the one from LinkedIn has no new release since 2020). At that points, I really have 2 options:

  1. find a working plugin with gradle that will work with python (which is current approach)
  2. use other approach outside gradle and enforce it with GH action (which then I found out that already existed...side note, I raised an enhancement PR for this approach: https://github.com/apache/polaris/pull/1954...if we really like one command from gradle, we can always have gradle run exec with make command from the enhancement PR)

With option one, i think the main benefit is people don't need to know another command to run (assuming everyone is happy with gradle...personally, makefile is a lot easier to use imo and we can create some very powerful makefile to control various tasks). But the down side comes to this plugin may got outdated or has different dependencies which can cause trouble for our existed build (which is the current case...checking what can be done).

That being said, I am fine with any of the approach and also open to new approach. Let me know what you think.

@HonahX
Copy link
Contributor

HonahX commented Jul 7, 2025

Thanks @eric-maynard and @MonkeyCanCode for the thorough explanations! I think the idea of adding convenient task in gradle is great. The questions lie more on the practical side that we seems do not have well-maintained/well functioned plugin to do so. Apart from the support problem of the current plugin, I am also a little bit concerned that this use-python plugin will use a virtual env (.gradle/python) other than one managed by poetry to run the code format/check. This means we are maintaining 2 dev python env in the codebase and may cause behavior deviation/confusion at some point.

Is it possible to make gradle task execute purely command line? Ideally the gradle task could be calling the Makefile target we define here #1995. In this way, we keep the MakeFile approach and also have convenient gradle tasks. Developers can choose whatever approaches they want and yet we only have a single dev environment and all deps are managed by poetry.

@MonkeyCanCode MonkeyCanCode marked this pull request as draft July 7, 2025 23:30
@MonkeyCanCode
Copy link
Contributor Author

As discussed on the other PR, I will convert this to draft mode. Once the makefile is updated to supports additional functionalities, we can revisit this one.

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.

3 participants