Skip to content

[Python][Client] Add LICENSE and NOTICE to CLI.#3891

Open
HonahX wants to merge 4 commits intoapache:mainfrom
HonahX:jojiang-python-cli-license-and-notice
Open

[Python][Client] Add LICENSE and NOTICE to CLI.#3891
HonahX wants to merge 4 commits intoapache:mainfrom
HonahX:jojiang-python-cli-license-and-notice

Conversation

@HonahX
Copy link
Contributor

@HonahX HonahX commented Feb 26, 2026

Add LICENSE and NOTICE file for python client. Python Client is supposed to distribute sdist and wheel. In sdist, we have the following files that are copied from other repo and thus included in the LICENSE

  • spec/iceberg-rest-catalog-open-api.yaml
  • spec/polaris-catalog-apis/oauth-tokens-api.yaml

Checklist

  • 🛡️ Don't disclose security issues! (contact security@apache.org)
  • 🔗 Clearly explained why the changes are needed, or linked related issues: Fixes #
  • 🧪 Added/updated tests with good coverage, or manually tested (and explained how)
  • 💡 Added comments for complex logic
  • 🧾 Updated CHANGELOG.md (if needed)
  • 📚 Updated documentation in site/content/in-dev/unreleased (if needed)

@github-project-automation github-project-automation bot moved this to PRs In Progress in Basic Kanban Board Feb 26, 2026
@HonahX HonahX marked this pull request as ready for review February 26, 2026 01:15

Copyright: Copyright 2017-2025 The Apache Software Foundation
Home page: https://iceberg.apache.org
License: https://www.apache.org/licenses/LICENSE-2.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to include the following dependencies? cc @jbonofre

dependencies = [
    "urllib3>=1.25.3,<3.0.0",
    "python-dateutil>=2.8.2",
    "pydantic>=2.12.5,<2.13.0",
    "typing-extensions>=4.7.1",
    "boto3~=1.42.2",
]

Copy link
Member

Choose a reason for hiding this comment

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

The LICENSE here should document what is in the product/artifact.

Considering the Python Client whl:

  1. the dist info METADATA described the dependencies resolved at execution:
Requires-Dist: boto3 (>=1.40.6,<1.41.0)
Requires-Dist: pydantic (>=2.0.0,<2.12.0)
Requires-Dist: python-dateutil (>=2.8.2)
Requires-Dist: typing-extensions (>=4.7.1)
Requires-Dist: urllib3 (>=1.25.3,<3.0.0)
  1. There's not code coming from anywhere (even Iceberg).
  2. The spec OpenAPI yaml are not included at all in the distributed artifact. It's used to generate the stubs.

So, actually, the LICENSE should not contain anything because the distributed artifact (product) doesn't bundle anything.

Copy link
Contributor

Choose a reason for hiding this comment

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

Great. I like how pypi distributes it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for reviewing! One follow-up question to confirm, the spec files are not included in the wheel but they do exist in the sdist, since for user install via sdist they will need to re-generate the client using the included spec

tar tzf client/python/dist/apache_polaris-1.4.0.tar.gz | grep "^apache_polaris-1.4.0/spec/"
apache_polaris-1.4.0/spec/README.md
apache_polaris-1.4.0/spec/iceberg-rest-catalog-open-api.yaml
apache_polaris-1.4.0/spec/polaris-catalog-service.yaml
apache_polaris-1.4.0/spec/polaris-management-service.yml
apache_polaris-1.4.0/spec/generated/bundled-polaris-catalog-service.yaml
apache_polaris-1.4.0/spec/polaris-catalog-apis/generic-tables-api.yaml
apache_polaris-1.4.0/spec/polaris-catalog-apis/notifications-api.yaml
apache_polaris-1.4.0/spec/polaris-catalog-apis/oauth-tokens-api.yaml
apache_polaris-1.4.0/spec/polaris-catalog-apis/policy-apis.yaml

Do we need to include spec files in the LICENSE in this case?

We could also only publish wheel since the wheel is universal

Copy link
Contributor

Choose a reason for hiding this comment

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

I thought you did that already in line 207 and 208

Copy link
Member

Choose a reason for hiding this comment

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

I checked in the wheel distribution.

So, if we plan to publish both wheel and a tar.gz distributions, yes, spec should be documented in LICENSE. Maybe easier to have the same layout between wheel and tar.gz distributions.

If we don't actually need the spec in the tar.gz distribution, I would propose to not include spec here.

I think we have three options:

  1. Don't include spec in tar.gz distribution
  2. Include spec in wheel distribution
  3. Only publish wheel distribution (not sure tar.gz distribution is very used)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestions! Spec are required in tar.gz distribution since pip install from source need to generate open api client from them.

Since we have universal wheel here that should cover all most of the cases, for simplicity let's target at option 3: only publish wheel distribution first. We could add sdist later if really needed.

cc: @MonkeyCanCode

Copy link
Member

@jbonofre jbonofre left a comment

Choose a reason for hiding this comment

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

LICENSE should not contain Iceberg because the distribution/product doesn't include (bundle) any code from Iceberg.
The copyright year in the NOTICE should be updated to 2026.


Copyright: Copyright 2017-2025 The Apache Software Foundation
Home page: https://iceberg.apache.org
License: https://www.apache.org/licenses/LICENSE-2.0
Copy link
Member

Choose a reason for hiding this comment

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

The LICENSE here should document what is in the product/artifact.

Considering the Python Client whl:

  1. the dist info METADATA described the dependencies resolved at execution:
Requires-Dist: boto3 (>=1.40.6,<1.41.0)
Requires-Dist: pydantic (>=2.0.0,<2.12.0)
Requires-Dist: python-dateutil (>=2.8.2)
Requires-Dist: typing-extensions (>=4.7.1)
Requires-Dist: urllib3 (>=1.25.3,<3.0.0)
  1. There's not code coming from anywhere (even Iceberg).
  2. The spec OpenAPI yaml are not included at all in the distributed artifact. It's used to generate the stubs.

So, actually, the LICENSE should not contain anything because the distributed artifact (product) doesn't bundle anything.

@MonkeyCanCode
Copy link
Contributor

Thanks for working on this @HonahX . Looks good to me. Once comments are addressed, I will approve.

@HonahX HonahX requested review from flyrain and jbonofre March 4, 2026 00:37
@github-project-automation github-project-automation bot moved this from PRs In Progress to Ready to merge in Basic Kanban Board Mar 4, 2026
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.

4 participants