Skip to content

[http-client-python] add pyproject toml #7829

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 35 commits into
base: main
Choose a base branch
from

Conversation

swathipil
Copy link
Member

@swathipil swathipil commented Jul 3, 2025

fix #7975
Changes to http-client-python:

  • Automatically generate with pyproject.toml instead of setup.py
  • Added an additional emitter option generate-setup-py to revert to setup.py generation if needed.

Copy link
Contributor

github-actions bot commented Jul 3, 2025

All changed packages have been documented.

  • @typespec/http-client-python
Show changes

@typespec/http-client-python - feature ✏️

Adding pyproject.toml generation and optional generate-setup-py flag

@azure-sdk
Copy link
Collaborator

azure-sdk commented Jul 3, 2025

You can try these changes here

🛝 Playground 🌐 Website 🛝 VSCode Extension

@swathipil swathipil requested a review from Copilot July 3, 2025 23:09
Copy link
Contributor

@Copilot 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 replaces setup.py with a pyproject.toml-based build for the Python HTTP client generator, adds a corresponding template and retention logic, and updates related packaging dependencies.

  • Added pyproject.toml.jinja2 template and support in the serializer
  • Updated serializer to remove setup.py, retain existing pyproject.toml fields, and manage version comparisons
  • Adjusted package.json to include new dependencies for Python client generation

Reviewed Changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/http-client-python/package.json Added new dependencies and reordered existing ones for Python codegen
generator/pygen/codegen/templates/.../pyproject.toml.jinja2 Introduced pyproject.toml template for packaging
generator/pygen/codegen/serializers/general_serializer.py Added methods to extract and retain pyproject.toml fields, removed setup.py code paths
generator/pygen/codegen/serializers/init.py Swapped out setup.py.jinja2 for pyproject.toml.jinja2, updated regeneration logic
generator/pygen/init.py Added remove_file helper to delete setup.py
cspell.yaml Added pytyped to the spelling dictionary
Files not reviewed (1)
  • packages/http-client-python/package-lock.json: Language not supported
Comments suppressed due to low confidence (2)

packages/http-client-python/generator/pygen/codegen/serializers/general_serializer.py:63

  • The logic for retaining existing pyproject.toml fields isn't covered by tests. Add unit tests for _keep_pyproject_fields to ensure dependencies and build-tool settings are correctly preserved.
    def _keep_pyproject_fields(self, file_path: str) -> dict:

packages/http-client-python/generator/pygen/codegen/templates/packaging_templates/pyproject.toml.jinja2:86

  • There is no newline between the closing {% endif %} and the readme line, which will produce invalid TOML. Add a line break so readme = ... appears on its own line.
{% endif %}readme = {file = ["README.md"], content-type = "text/markdown"}

@swathipil swathipil requested review from lmazuel and iscai-msft July 3, 2025 23:51
Copy link
Contributor

@msyyc msyyc left a comment

Choose a reason for hiding this comment

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

@msyyc
Copy link
Contributor

msyyc commented Jul 21, 2025

I notice codegen will generate pyproject.toml instead of setup.py after this PR. What about the existing SDKs? Is it OK when both setup.py and pyproject.toml exist at the same time? Shall we delete the existing setup.py when new pyproject.toml generated in codegen?

@swathipil
Copy link
Member Author

swathipil commented Jul 21, 2025

@msyyc - Looking into the bug where pyproject.toml isn't being generated for swagger.

For existing SDKs, it should regenerate with pyproject.toml and remove the existing setup.py file. If any existing SDKs need to continue using setup.py for some reason, they can set generate-setup-py: true in the tspconfig.yaml.

@msyyc msyyc self-requested a review July 22, 2025 02:34
Comment on lines +73 to +75
# Keep azure-sdk-build configuration
if "tool" in loaded_pyproject_toml and "azure-sdk-build" in loaded_pyproject_toml["tool"]:
result["KEEP_FIELDS"]["tool.azure-sdk-build"] = loaded_pyproject_toml["tool"]["azure-sdk-build"]
Copy link
Contributor

@msyyc msyyc Jul 22, 2025

Choose a reason for hiding this comment

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

@swathipil I try to use this PR to regenerate for existing SDKs and find some content of pyproject.toml is deleted like https://github.com/Azure/azure-sdk-for-python/compare/main...msyyc:test-2025-07-22?expand=1

image

It seems that the logic doesn't work.

Comment on lines +138 to +144
# remove setup.py file
if self.code_model.options.get("generate-setup-py") or self.code_model.options.get("basic-setup-py"):
# Write the setup file
if self.code_model.options["basic-setup-py"]:
self.write_file(exec_path / Path("setup.py"), general_serializer.serialize_setup_file())
else:
self.remove_file(exec_path / Path("setup.py"))
Copy link
Contributor

Choose a reason for hiding this comment

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

@swathipil It is not intuitive for the judge logic. It is better like:

if basic-setup-py:
  # write setup
elif self.code_model.options.get("generate-setup-py") is False:
  # remove setup.py

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.

[python][feature] add pyproject.toml as default packaging file
3 participants