Skip to content

Conversation

@MonkeyCanCode
Copy link
Contributor

@MonkeyCanCode MonkeyCanCode commented Jul 28, 2025

This is a follow up PR based on #2049. To give more context, currently we are using client/templates/regenerate.sh to regenerate code for python cli from openapi within docker. Then on gradle side, we have a CMD registers as regeneratePythonClient to provide user to regenerate the python code.

Based on the discussion from above PR as well as the experimental PR (#1885) from @HonahX, I then raised this PR to move python code generation away from gradle and make it as part of poetry and remove dependency for docker.

Unlikes the experimental PR, this PR has the following as extra:

  1. ensure openapi only generate source code and another else (the one in the experimental PR will setup a bunch files and try to modify many existed files as well including pyproject.toml and README.md)
  2. makefile and polaris bash script are updated to not depends on gradle for client code generation
  3. remove client/templates/regenerate.sh as that is no longer needed (all functionalities are ported to the client/python/generate_clients.py)
  4. copy header templates into docker for testing
  5. minor fix in the Docker file with &&

@MonkeyCanCode
Copy link
Contributor Author

@snazy one of the efforts we talked about in the other PR where we will be moving away from gradle for non-java based workload such as python client related.

@flyrain flyrain requested a review from eric-maynard August 6, 2025 00:43
@HonahX HonahX self-requested a review August 7, 2025 01:38
Copy link
Contributor

@HonahX HonahX left a comment

Choose a reason for hiding this comment

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

@MonkeyCanCode Thanks for finalizing the auto-generation solution, I think this will also make it easier for us to publish the package in the future.

print(f"Error prepending header to {file_path}: {e}")


def prepend_licenses() -> None:
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if we need to add license header to auto-generated codes that are not tracked by git, but we can preserve the existing behavior and make a decision later

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah. I would suggest to keep the same then clean up those in the following PR. What do u think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had to revert the change for not auto generated doc. Seems to be causing issues on python 3.9 during build (i am on python 3.13 and no issue there). I can revisit this one later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PR: #2439

@github-project-automation github-project-automation bot moved this from PRs In Progress to Ready to merge in Basic Kanban Board Aug 7, 2025
Copy link
Contributor

@HonahX HonahX left a comment

Choose a reason for hiding this comment

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

I think we also need to update the scripts in getting-started examples, otherwise it will break

./gradlew regeneratePythonClient

polaris/spec/README.md

Lines 54 to 56 in 5983c81

However, when copying it, you may need to make some nonfunctional changes in order to ensure that the Python types
generated by the gradle task `regeneratePythonClient` still allow all tests to pass. For more context, see
[PR #1347](https://github.com/apache/polaris/pull/1347).

Copy link
Contributor

@HonahX HonahX left a comment

Choose a reason for hiding this comment

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

Temporarily block merging to prevent breaking getting-started examples. Everything else LGTM!

@github-project-automation github-project-automation bot moved this from Ready to merge to PRs In Progress in Basic Kanban Board Aug 7, 2025
@MonkeyCanCode
Copy link
Contributor Author

I think we also need to update the scripts in getting-started examples, otherwise it will break

./gradlew regeneratePythonClient

polaris/spec/README.md

Lines 54 to 56 in 5983c81

However, when copying it, you may need to make some nonfunctional changes in order to ensure that the Python types
generated by the gradle task `regeneratePythonClient` still allow all tests to pass. For more context, see
[PR #1347](https://github.com/apache/polaris/pull/1347).

will do.

@MonkeyCanCode
Copy link
Contributor Author

I think we also need to update the scripts in getting-started examples, otherwise it will break

./gradlew regeneratePythonClient

polaris/spec/README.md

Lines 54 to 56 in 5983c81

However, when copying it, you may need to make some nonfunctional changes in order to ensure that the Python types
generated by the gradle task `regeneratePythonClient` still allow all tests to pass. For more context, see
[PR #1347](https://github.com/apache/polaris/pull/1347).

will do.

This is resolved.

HonahX
HonahX previously approved these changes Aug 8, 2025
Copy link
Contributor

@HonahX HonahX left a comment

Choose a reason for hiding this comment

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

Thanks for updating !

@github-project-automation github-project-automation bot moved this from PRs In Progress to Ready to merge in Basic Kanban Board Aug 8, 2025
@snazy snazy added this to the 1.1.0 milestone Aug 20, 2025
@snazy
Copy link
Member

snazy commented Aug 20, 2025

Anything left on this PR?

@MonkeyCanCode MonkeyCanCode merged commit 9c455ed into apache:main Aug 20, 2025
12 of 13 checks passed
@github-project-automation github-project-automation bot moved this from Ready to merge to Done in Basic Kanban Board Aug 20, 2025
snazy added a commit to snazy/polaris that referenced this pull request Nov 20, 2025
* feat: Add Pod Disruption Budget support to Helm chart (apache#2380)

* chore(deps): update quay.io/keycloak/keycloak docker tag to v26.3.3 (apache#2407)

* Mention Helm chart support for PodDisruptionBudget in CHANGELOG.md (apache#2408)

* chore: Suppress javac deprecation warnings in SparkCatalog (apache#2394)

SparkCatalog intentionally overrides and uses deprecated
methods from Spark's TableCatalog.

This PR adds suppression annotations to allow for clean
compilation given that the deprecated method calls and
overrides are clearly expected in this case.

* Python client auto generate (apache#2192)

* Python client auto generate

* Python client auto generate

* Python client auto generate

* Python client auto generate

* Python client auto generate

* Python client auto generate

* Remove auto generated doc

* undo

* Fix doc

* Fix docker ref from CONTAINER_TOOL to DOCKER

* Add client help manual to GH action

* Add missing region to MinIO getting-started example (apache#2411)

The example was missing an AWS region, thus causing Spark to fail with:

```
spark-sql ()> create table ns.t1 as select 'abc';
25/08/20 16:25:06 ERROR Executor: Exception in task 0.0 in stage 0.0 (TID 0)
software.amazon.awssdk.core.exception.SdkClientException: Unable to load region from any of the providers in the chain software.amazon.awssdk.regions.providers.DefaultAwsRegionProviderChain@47578c86: [software.amazon.awssdk.regions.providers.SystemSettingsRegionProvider@1656f847: Unable to load region from system settings. Region must be specified either via environment variable (AWS_REGION) or  system property (aws.region)., software.amazon.awssdk.regions.providers.AwsProfileRegionProvider@2bbaabe3: No region provided in profile: default, software.amazon.awssdk.regions.providers.InstanceProfileRegionProvider@54b1cfd8: Unable to contact EC2 metadata service.]
...
        at org.apache.iceberg.aws.AwsClientFactories$DefaultAwsClientFactory.s3(AwsClientFactories.java:119)
        at org.apache.iceberg.aws.s3.S3FileIO.client(S3FileIO.java:391)
        at org.apache.iceberg.aws.s3.S3FileIO.newOutputFile(S3FileIO.java:193)
```

* Add feature config to allow dropping views without purging (apache#2369)

* Add feature config to allow dropping views without purging

With tables, the client can decide whether to purge the table
on drop or not. However, Polaris Servers used to unconditionally
perform the purge on dropping a view.

After apache#1619 that behaviour effectively prevents dropping views
if the admin user does not set `DROP_WITH_PURGE_ENABLED`. The
latter, though, is not currently advisable per apache#1617.

This change introduces a new feature configuration
(`PURGE_VIEWS_ON_DROP`) that allows the admin user to instruct
Polaris servers to drop views without purging to achieve
operational parity with tables.

Fixes apache#2367

* review: rename to PURGE_VIEW_METADATA_ON_DROP

* review: re-fix description

* Last merged commit c97b150

---------

Co-authored-by: Bryan Maloyer <bryan.mlyr@gmail.com>
Co-authored-by: Mend Renovate <bot@renovateapp.com>
Co-authored-by: Alexandre Dutra <adutra@apache.org>
Co-authored-by: Dmitri Bourlatchkov <dmitri.bourlatchkov@gmail.com>
Co-authored-by: Yong Zheng <yongzheng0809@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants