python: add type hints for project proxy - #4255
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces comprehensive type hinting and data structures, including Enums and TypedDicts, to the ProjectProxy class. It also cleans up docstrings by removing redundant type information. The review feedback highlights several necessary improvements: PermissionState and OrderType enums should inherit from str to ensure proper JSON serialization and query parameter formatting. Furthermore, the return types for get_list and search should be updated to List[Munch], and a new PermissionRequest TypedDict should be used for the request_permissions method to correctly represent the expected boolean values.
|
|
||
|
|
||
| def _compat_use_bootstrap_container(data, value): | ||
| class PermissionState(Enum): |
There was a problem hiding this comment.
The PermissionState enum should inherit from str (i.e., class PermissionState(str, Enum):). Since this enum is used in TypedDict structures that are eventually passed to requests as JSON data (e.g., in set_permissions), it needs to be JSON serializable. In Python 3.6, standard Enum objects are not serializable by the default json encoder, which will lead to a TypeError at runtime.
| class PermissionState(Enum): | |
| class PermissionState(str, Enum): |
| UserPermissions = Dict[str, Permissions] | ||
|
|
||
|
|
||
| class OrderType(Enum): |
There was a problem hiding this comment.
The OrderType enum should inherit from str (i.e., class OrderType(str, Enum):). This enum is used in PaginationMeta, which is passed as query parameters via params.update(). Standard Enum objects do not automatically convert to their value string when used in requests query parameters (they typically stringify to something like 'OrderType.ASC'), which would likely cause the API to fail to recognize the sort order.
| class OrderType(Enum): | |
| class OrderType(str, Enum): |
| class Permissions(TypedDict, total=False): | ||
| builder: PermissionState | ||
| admin: PermissionState |
There was a problem hiding this comment.
The Permissions TypedDict is correctly defined for set_permissions (where states are strings like "approved"), but it is not suitable for request_permissions. According to the docstring for request_permissions, that method expects boolean values (True to request, False to drop). I suggest defining a separate PermissionRequest TypedDict for that purpose.
| class Permissions(TypedDict, total=False): | |
| builder: PermissionState | |
| admin: PermissionState | |
| class Permissions(TypedDict, total=False): | |
| builder: PermissionState | |
| admin: PermissionState | |
| class PermissionRequest(TypedDict, total=False): | |
| builder: bool | |
| admin: bool |
| self, | ||
| ownername: Optional[str] = None, | ||
| pagination: Optional[PaginationMeta] = None, | ||
| ) -> Munch: |
There was a problem hiding this comment.
The return type for get_list should be List[Munch] rather than Munch. While munchify returns a single Munch for single-object responses, for list responses it returns a custom List object (from copr.v3.helpers) which behaves like a list of Munch objects. Using List[Munch] provides a more accurate hint for users who intend to iterate over the results.
| ) -> Munch: | |
| ) -> List[Munch]: |
| self, | ||
| query: str, | ||
| pagination: Optional[PaginationMeta] = None, | ||
| ) -> Munch: |
There was a problem hiding this comment.
Similar to get_list, the search method returns a list of projects. The return type should be updated to List[Munch] to accurately reflect that the result is an iterable collection of project objects.
| ) -> Munch: | |
| ) -> List[Munch]: |
| self, | ||
| ownername: str, | ||
| projectname: str, | ||
| permissions: Permissions, |
There was a problem hiding this comment.
As noted previously, request_permissions expects boolean values for the permissions, not the PermissionState enum values. Please use the suggested PermissionRequest TypedDict here.
| permissions: Permissions, | |
| permissions: PermissionRequest, |
579e40e to
3a2ef5b
Compare
Oh. I wouldn't expect these modern clever AI tools require us to be that "deterministic" and "explicit". |
If you don't require type annotation for older epel, you can get away with |
|
(I'm not fan of this, at all! but...) how is this PR going? |
|
I promised I would test this doesn't break |
3a2ef5b to
24eae4b
Compare
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughProject proxy declarations now define typed permission, ordering, pagination, and user-permission structures. All public ChangesProject proxy typing
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@python/copr/v3/proxies/project.py`:
- Line 151: Update the runtime_dependencies parameter annotations in the
affected project proxy methods to use an optional list type matching the
documented external-repository payload instead of Optional[str], and keep the
default value as None.
- Around line 86-90: Update the return annotations of both paginated get_list
methods to use the actual pagination response type rather than Munch. If no
concrete type exists, define and use a protocol covering items, meta, and
response attributes, matching the custom List returned by munchify.
- Around line 493-498: Update the permissions payload type used by
request_permissions so builder and admin accept boolean values as documented,
replacing the PermissionState-based Permissions type with the appropriate
boolean-valued type while preserving the method’s existing behavior.
- Around line 14-41: Make PermissionState and OrderType string-backed enums by
inheriting from str and Enum, ensuring their values serialize as strings in
request JSON payloads and query parameters. Preserve the existing enum members
and values so Permissions payloads and get_list/search ordering continue using
the same API values.
In `@python/setup.py`:
- Line 22: Update the typing_extensions dependency in setup.py to require a
minimum version that provides TypedDict, such as typing_extensions>=3.7.4, while
preserving the existing dependency declaration and Python 3.6 support.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 59e187b8-2b38-419d-83c4-081b3e1f5752
📒 Files selected for processing (3)
python/copr/v3/proxies/project.pypython/python-copr.specpython/setup.py
| class PermissionState(Enum): | ||
| """ | ||
| Possible values that user can have or set for their `builder` and `admin` | ||
| permissions on some project. | ||
| """ | ||
| NOTHING = "nothing" | ||
| REQUEST = "request" | ||
| APPROVED = "approved" | ||
|
|
||
|
|
||
| class Permissions(TypedDict, total=False): | ||
| """ | ||
| A set of permissions that a user has or wants to have for some project | ||
| """ | ||
| builder: PermissionState | ||
| admin: PermissionState | ||
|
|
||
|
|
||
| # The `str` keys are usernames | ||
| UserPermissions = Dict[str, Permissions] | ||
|
|
||
|
|
||
| class OrderType(Enum): | ||
| """ | ||
| Order items in ascending or descending order | ||
| """ | ||
| ASC = "ASC" | ||
| DESC = "DESC" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline python/copr/v3/requests.py --items all --match 'Request|send'
rg -n -C 8 'def send|Enum|\.value|params=|data=' python/copr/v3/requests.pyRepository: fedora-copr/copr
Length of output: 2117
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the relevant Request.send request-assembly code and project call sites without running repo code.
sed -n '1,90p' python/copr/v3/requests.py
printf '\n--- project relevant symbols ---\n'
rg -n -C 5 'set_permissions|request_permissions|get_list|search|PermissionState|OrderType|permissions=|pagination=|order=|permission' python/copr/v3/project.py python/copr/v3/proxies/project.pyRepository: fedora-copr/copr
Length of output: 16798
🌐 Web query:
requests json argument json module dumps Enum member Python
💡 Result:
The Python requests library's json parameter accepts any object that can be serialized into JSON [1][2][3]. However, standard Python enum.Enum members are not natively JSON serializable by the json module and will raise a TypeError if passed directly [4][5][6]. To include an Enum member in the json parameter of a requests call, you must first convert it to a serializable type (like a string, integer, or dictionary) [5][7][8]. Common Approaches: 1. Use Enum values: If your Enum is simple, you can pass the member's value directly [8]. requests.post(url, json={"status": MyEnum.ACTIVE.value}) 2. Use the default argument: If you are serializing a larger structure, you can use the default argument in json.dumps to specify how to handle non-serializable objects like Enums before passing them to requests [5][7]. import json import requests data = {"status": MyEnum.ACTIVE} json_data = json.dumps(data, default=lambda x: x.value) requests.post(url, data=json_data, headers={"Content-Type": "application/json"}) 3. Use IntEnum or StrEnum: If your Enum inherits from int or str (e.g., class Status(str, Enum):...), the Enum members are often treated as their base types and can be serialized automatically by the json module [4][5][8]. Note that requests' json parameter performs the serialization internally using json.dumps [9]. Since requests does not expose a way to pass a custom cls (encoder) or default function directly to its internal json serializer, you must manually serialize your data to a JSON string using json.dumps (with your custom logic) and pass it to the data parameter instead, or ensure your data structure consists only of types natively supported by json.dumps [1][4][10][7].
Citations:
- 1: https://docs.python-requests.org/en/latest/api/?highlight=codes
- 2: https://requests.readthedocs.io/en/latest/api/index.html
- 3: https://docs.python-requests.org/en/latest/api/?highlight=json
- 4: https://docs.python.org/3/library/json.html
- 5: https://stackoverflow.com/questions/24481852/serialising-an-enum-member-to-json
- 6: https://stackoverflow.com/questions/36699512/is-it-possible-to-dump-an-enum-in-json-without-passing-an-encoder-to-json-dumps
- 7: https://sqlpey.com/python/solved-how-to-serialize-and-deserialize-python-enum-members-to-json/
- 8: https://engineersofai.com/docs/python/python-intermediate/apis-and-web-basics/json-serialization
- 9: https://github.com/psf/requests/blob/4f6c0187150af09d085c03096504934eb91c7a9e/requests/api.py
- 10: https://exchangetuts.com/index.php/how-to-json-serialize-enum-classes-in-pydantic-basemodel-1766255403394084
🌐 Web query:
requests params argument json module dumps Enum member Python
💡 Result:
When using Python's requests library and the json module with Enum members, there are two distinct scenarios to consider: sending URL parameters (params) and sending a JSON request body (json). URL Parameters (params) The requests library's params argument is designed to handle query string parameters [1][2]. While the library officially supports strings, mappings of strings, or iterables of two-tuples [3], it often implicitly handles non-string objects (like Enums) by converting them to strings via the underlying URL encoding process [3]. However, this behavior is an implementation detail and is not explicitly guaranteed as a feature [3]. If you need specific string representations for your Enum members in the URL, it is best practice to explicitly convert them to their string or value representation (e.g., member.value or member.name) before passing them to params [3]. JSON Request Body (json) The json argument in requests uses the standard library's json.dumps function [4][5]. 1. Int- and float-derived Enums (e.g., IntEnum) are supported natively by json.dumps and will be serialized as their numeric values [4]. 2. Standard Enum types (where the member is not an int or float) are not automatically serializable by json.dumps and will raise a TypeError [4][6]. To handle standard Enum serialization: - Use a custom encoder by subclassing json.JSONEncoder and overriding the default method [4][5]. - Alternatively, if you want your Enum members to behave like strings in JSON, you can inherit from both str and Enum (e.g., class MyEnum(str, Enum): ...), which ensures they serialize as their string values natively [6][7].
Citations:
- 1: https://docs.python-requests.org/en/latest/api/
- 2: https://github.com/psf/requests/blob/main/src/requests/api.py
- 3: Can querystring params be non-strings? psf/requests#3856
- 4: https://docs.python.org/3/library/json.html
- 5: https://github.com/python/cpython/blob/main/Lib/json/encoder.py
- 6: https://stackoverflow.com/questions/36699512/is-it-possible-to-dump-an-enum-in-json-without-passing-an-encoder-to-json-dumps
- 7: https://stackoverflow.com/questions/43854335/encoding-python-enum-to-json
Encode enum values before sending request payloads.
Request.send passes data directly as the json argument for requests, and plain PermissionState enum members are not JSON-serializable through the normal requests request flow. Convert these values to strings before sending, for example by making PermissionState(str, Enum) or normalizing .value before calling send. OrderType is a similar string enum used through get_list/search; ensure params serialize it as its string value too.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@python/copr/v3/proxies/project.py` around lines 14 - 41, Make PermissionState
and OrderType string-backed enums by inheriting from str and Enum, ensuring
their values serialize as strings in request JSON payloads and query parameters.
Preserve the existing enum members and values so Permissions payloads and
get_list/search ordering continue using the same API values.
| def get_list( | ||
| self, | ||
| ownername: Optional[str] = None, | ||
| pagination: Optional[PaginationMeta] = None, | ||
| ) -> Munch: |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Correct the paginated return types.
munchify returns the custom paginated List when the response contains items. Both endpoints return that shape. -> Munch gives typed callers the wrong public contract.
Use the actual pagination response type, or define a protocol for its items, meta, and response attributes.
Also applies to: 106-110
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@python/copr/v3/proxies/project.py` around lines 86 - 90, Update the return
annotations of both paginated get_list methods to use the actual pagination
response type rather than Munch. If no concrete type exists, define and use a
protocol covering items, meta, and response attributes, matching the custom List
returned by munchify.
| 'requests-toolbelt', | ||
| 'setuptools', | ||
| 'munch', | ||
| 'typing_extensions', |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
venv_dir="$(mktemp -d)"
trap 'rm -rf "$venv_dir"' EXIT
python3 -m venv "$venv_dir"
"$venv_dir/bin/pip" install --quiet 'typing_extensions==3.6.6'
if "$venv_dir/bin/python" -c 'from typing_extensions import TypedDict'; then
echo "Unexpected TypedDict support in typing_extensions==3.6.6"
exit 1
fiRepository: fedora-copr/copr
Length of output: 290
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== repo files =="
git ls-files | rg '(^|/)python/setup\.py$|(^|/)project\.py$' || true
echo "== setup.py relevant lines =="
if [ -f python/setup.py ]; then
nl -ba python/setup.py | sed -n '1,80p'
fi
echo "== imports of typing_extensions =="
rg -n "from typing_extensions import|import typing_extensions\.TypedDict|TypedDict" python || trueRepository: fedora-copr/copr
Length of output: 324
🌐 Web query:
typing-extensions 3.6.6 TypedDict support documentation
💡 Result:
The typing-extensions package version 3.6.6 does not include support for TypedDict [1]. TypedDict was added to the typing_extensions library in subsequent versions [2]. While development and early integration occurred around 2019 [3][4], the stable, widely-documented backporting functionality for TypedDict that developers typically look for in typing_extensions evolved significantly in versions following 3.6.6 [5][6]. For projects requiring TypedDict support, it is recommended to use a modern version of typing_extensions, as the implementation has been continuously updated to align with the standard library's TypedDict behavior and to include critical bug fixes across different Python versions [5][6][7]. As of the current date, official documentation for typing_extensions provides detailed guidance on TypedDict, noting that it mirrors functionality from the standard library (introduced in Python 3.8) while providing necessary backports and improvements for older or specific Python environments [5][8].
Citations:
- 1: https://pypi.org/project/typing-extensions/3.6.6/
- 2: https://www.suse.com/support/update/announcement/2020/suse-ru-20201567-2/
- 3: Add TypedDict to typing_extensions python/typing#628
- 4: Add typing_extensions module for new / optional typing features to PyPI python/typing#435
- 5: https://typing-extensions.readthedocs.io/en/stable/
- 6: https://typing-extensions.readthedocs.io/en/stable/index.html
- 7: https://github.com/python/typing%5Fextensions/blob/main/CHANGELOG.md
- 8: https://typing-extensions.readthedocs.io/en/latest/?featured_on=pythonbytes
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== setup.py relevant lines =="
awk 'NR>=1 && NR<=80 { printf "%6d\t%s\n", NR, $0 }' python/setup.py
echo "== project.py relevant lines =="
awk 'NR>=1 && NR<=220 { printf "%6d\t%s\n", NR, $0 }' python/copr/v3/proxies/project.py
echo "== all TypedDict usage =="
rg -n "TypedDict" python || trueRepository: fedora-copr/copr
Length of output: 11242
🌐 Web query:
typing-extensions PyPI 3.6.7 3.7.0 TypedDict added
💡 Result:
TypedDict was added to the typing_extensions library in version 3.7.4. Prior to this, TypedDict was introduced to the Python standard library in Python 3.8 via PEP 589. The typing_extensions package provided TypedDict as an experimental feature to backport this functionality to earlier Python versions (such as 3.6 and 3.7). While typing_extensions version 3.7.4.1 (released in 2019) is commonly cited in historical discussions regarding this backport, it was part of the 3.7.4 release cycle where TypedDict support was solidified. Summary of key details: - Introduction: TypedDict was added to typing_extensions in the 3.7.4 release series to support PEP 589 features on Python versions older than 3.8 [1][2][3]. - Purpose: It served as a backport for users on Python 3.6+ who needed TypedDict functionality before it was available in the standard library or to provide enhanced features not present in older standard library implementations [1][4][5]. - Evolution: Since its addition, typing_extensions has continuously updated its TypedDict implementation to match or improve upon the standard library, including changes such as converting it from a class to a function in version 4.7.0 and adding support for newer PEPs like 728 [6][7][5][8].
Citations:
- 1: https://pypi.org/project/typing-extensions/3.7.4.1/
- 2: Trying to import TypedDict from typing of tpying_extensions fails python/typeshed#3500
- 3: https://pypi.loongnix.cn/loongson/pypi/typing-extensions/3.7.4.3
- 4: https://github.com/python/typing_extensions/blob/main/doc/index.rst
- 5: https://typing-extensions.readthedocs.io/en/latest/
- 6: https://github.com/python/typing_extensions/blob/4.15.0/CHANGELOG.md
- 7: https://github.com/python/typing_extensions/releases/tag/4.7.0
- 8: https://github.com/python/typing_extensions/blob/f02b99d3/doc/index.rst
🌐 Web query:
pypi typing-extensions versions 3.6.6 3.6.7 3.7.0 type metadata
💡 Result:
The PyPI project typing-extensions tracks the following information regarding the requested versions: - Version 3.6.6: This version was released on October 6, 2018 [1][2]. It was distributed as a source distribution (sdist) [1] and as wheel files (py2-none-any and py3-none-any) [3][4]. - Version 3.6.7: There is no record of a version 3.6.7 in the PyPI version history for typing-extensions [1]. - Version 3.7.0: There is no record of a version 3.7.0 in the PyPI version history for typing-extensions [1]. The release sequence transitioned from 3.6.6 to 3.7.2 (released January 12, 2019) [1][2]. Regarding "type metadata," as PyPI packages, these versions include standard package metadata (such as author, license, and description) accessible via the PyPI JSON API or the package's landing page [5][1]. While typing-extensions provides backports and experimental type hints, it does not introduce unique "type metadata" formats beyond standard Python packaging specifications (e.g., METADATA files within the wheel or sdist) [5].
Citations:
- 1: https://pypi.org/project/typing-extensions/
- 2: https://app.soos.io/research/packages/Python/-/typing-extensions/
- 3: https://pypi.org/simple/typing-extensions/
- 4: https://mirrors.cloud.tencent.com/pypi/simple/typing-extensions/
- 5: https://pypi.org/project/typing-extensions/3.6.6/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== install_requires dependency declarations =="
rg -n "'typing_extensions'|typing_extensions" python/setup.py python -g '*.py' || true
echo "== package metadata classifiers =="
awk 'NR>=40 && NR<=48 { printf "%6d\t%s\n", NR, $0 }' python/setup.pyRepository: fedora-copr/copr
Length of output: 862
🌐 Web query:
GitHub python typing_extensions 3.7.4 TypedDict import
💡 Result:
In Python 3.7, the TypedDict type is not available in the standard library's typing module [1]. To use TypedDict in a Python 3.7 environment, you must install and import it from the typing_extensions package [2][1]. To use TypedDict, you should install the package: pip install typing-extensions Then, use the following import pattern to maintain compatibility across different Python versions [1]: import sys if sys.version_info >= (3, 8): from typing import TypedDict else: from typing_extensions import TypedDict The typing_extensions package provides backported features and improvements for type hinting, ensuring that newer features like TypedDict can be used in older Python versions [3][4][5]. While typing_extensions 3.7.4 is a specific legacy release [2][6], it is generally recommended to use the latest version of typing_extensions for ongoing projects to benefit from the most recent bug fixes and features [3][7].
Citations:
- 1: Trying to import TypedDict from typing of tpying_extensions fails python/typeshed#3500
- 2: https://pypi.org/project/typing-extensions/3.7.4/
- 3: https://github.com/python/typing_extensions/
- 4: https://github.com/python/typing_extensions/blob/main/doc/index.rst
- 5: https://typing-extensions.readthedocs.io/en/latest/
- 6: https://pypi.org/project/typing-extensions/3.7.4.3/
- 7: https://pypi.org/project/typing-extensions/
🌐 Web query:
Python typeshed typing_extensions TypedDict first version __all__
💡 Result:
The TypedDict construct was added to the typing_extensions library to provide a backport of the functionality defined in PEP 589 [1][2]. It was primarily introduced to allow users on Python versions earlier than 3.8 (where TypedDict was first added to the standard library) to utilize the feature [3][4]. Regarding its introduction and all inclusion: 1. Introduction: TypedDict was moved from the mypy_extensions project to typing_extensions to provide a unified, standardized interface for type checkers and users [3][2]. This addition was formalized in the typing_extensions repository (e.g., via pull request #628) [5]. 2. all inclusion: TypedDict is included in the all list of the typing_extensions module [6][7][8]. While the exact release version that first included it in all is not explicitly stated in the provided documentation, it has been a fixture of the typing_extensions library's public API for many years, following its migration from mypy_extensions [3][2]. The all list serves to explicitly export these typing primitives for public use [6][8]. For modern Python development, typing_extensions continues to backport bug fixes and newer features (such as PEP 705 and PEP 728 extensions) to TypedDict that may not yet be available in older versions of the standard library typing module [9][4].
Citations:
- 1: Add TypedDict to typing_extensions python/typeshed#2940
- 2: https://github.com/python/peps/blob/main/peps/pep-0589.rst
- 3: Move TypedDict from mypy_extensions to typing_extensions python/mypy#5288
- 4: https://typing-extensions.readthedocs.io/en/latest/?featured_on=pythonbytes
- 5: Add TypedDict to typing_extensions python/typing#628
- 6: https://github.com/python/typing_extensions/blob/main/src/typing_extensions.py
- 7: https://github.com/python/typing_extensions/blob/f02b99d3/src/typing_extensions.py
- 8: https://raw.githubusercontent.com/python/typing_extensions/main/src/typing_extensions.py
- 9: https://github.com/python/typing%5Fextensions/blob/main/CHANGELOG.md
🌐 Web query:
typing-extensions changelog TypedDict 3.7.4
💡 Result:
In typing-extensions version 3.7.4, TypedDict was introduced to the package [1][2]. This release, published in June 2019, added TypedDict as an experimental feature to support users on older Python versions, as it was not yet part of the standard library's typing module in earlier Python releases [3][4][2]. While the original changelog for 3.7.4 is not explicitly documented as a standalone text block in the modern repository, its introduction is widely recognized in technical documentation and project dependency history as the version that first enabled the import of TypedDict from typing_extensions [4][2]. Subsequent versions of typing-extensions have continued to update and align the implementation of TypedDict with the evolving standard library [5][6][7].
Citations:
- 1: https://pypi.org/project/typing-extensions/
- 2: build: Set lower bound of typing-extensions to v3.7.4 scikit-hep/pyhf#1938
- 3: https://pypi.org/project/typing-extensions/3.7.4/
- 4: https://tracker.debian.org/media/packages/p/python-typing-extensions/changelog-3.7.4.3-1
- 5: https://github.com/python/typing_extensions/blob/main/CHANGELOG.md
- 6: https://github.com/python/typing_extensions/blob/4.15.0/CHANGELOG.md
- 7: https://github.com/python/typing_extensions/releases/tag/4.7.0
Set a minimum typing_extensions version.
python/copr/v3/proxies/project.py imports TypedDict; package classifiers still include Python 3.6 support, while python/setup.py allows any satisfying release. Pin the dependency to a version that provides TypedDict, such as typing_extensions>=3.7.4.
Proposed fix
- 'typing_extensions',
+ 'typing_extensions>=3.7.4',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 'typing_extensions', | |
| 'typing_extensions>=3.7.4', |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@python/setup.py` at line 22, Update the typing_extensions dependency in
setup.py to require a minimum version that provides TypedDict, such as
typing_extensions>=3.7.4, while preserving the existing dependency declaration
and Python 3.6 support.
|
I tested this on AlmaLinux 8 with Python 3.6, and it works fine. |
24eae4b to
aa23453
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@python/copr/v3/proxies/project.py`:
- Around line 68-73: Update the classifiers in python/setup.py to remove the
Python 2 and Python 2.7 entries, since the annotated get method in the project
proxy indicates Python 3-only support. Preserve the existing Python 3
classifiers and other package metadata.
- Around line 84-93: Update the return annotations of project proxy methods
get_list and search to use a Python 3.6-compatible generic such as
typing.List[Munch] instead of the non-generic CoprList[Munch], preventing
module-import TypeError while preserving the paginated return behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f814e5a0-f183-42d5-92fa-ef596df92e97
📒 Files selected for processing (4)
python/copr/test/client_v3/test_projects.pypython/copr/v3/proxies/project.pypython/python-copr.specpython/setup.py
| def get(self, ownername: str, projectname: str) -> Munch: | ||
| """ | ||
| Return a project | ||
|
|
||
| :param str ownername: | ||
| :param str projectname: | ||
| :param ownername: | ||
| :param projectname: |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C2 'Programming Language :: Python :: 2|def get\(self, ownername: str' \
python/setup.py python/copr/v3/proxies/project.py
if command -v python2 >/dev/null 2>&1; then
python2 - <<'PY'
def project_get(ownername: str):
pass
PY
fiRepository: fedora-copr/copr
Length of output: 883
Drop Python 2 support from metadata.
python/copr/v3/proxies/project.py uses function annotations, so python/setup.py’s Programming Language :: Python :: 2 and Programming Language :: Python :: 2.7 classifiers are wrong. Remove those classifiers if Python 2 support is no longer intended.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@python/copr/v3/proxies/project.py` around lines 68 - 73, Update the
classifiers in python/setup.py to remove the Python 2 and Python 2.7 entries,
since the annotated get method in the project proxy indicates Python 3-only
support. Preserve the existing Python 3 classifiers and other package metadata.
aa23453 to
e220024
Compare
I know I have been the main antagonist to introducing type hints into our code, as debated for example here fedora-copr/debate#3 In the meantime, I tried them on personal projects and I quite like them but I am still on the fence whether they are worth it or not. However, our python client API is the one place where it is IMHO clear-cut - as evidenced by us putting the types into docstrings. Notice that I am only adding types to function parameters and returns, not their internal code. This commit changes only `ProjectProxy` so that we have an example. If we like it, I'll add type hints for the rest of the proxies as well. I don't use some of the modern type hinting features to remain compatible with EPEL8 and its Python 3.6, hence `List`, `Dict`, and `Optional`. The reason behind this effort is the Copr MCP server https://github.com/fedora-copr/copr-mcp It is supposed to produce the best results on typed functions.
e220024 to
7a61ce5
Compare
|
/packit test |
4 participants
I know I have been the main antagonist to introducing type hints into our code, as debated for example here
fedora-copr/debate#3
In the meantime, I tried them on personal projects and I quite like them but I am still on the fence whether they are worth it or not. However, our python client API is the one place where it is IMHO clear-cut - as evidenced by us putting the types into docstrings. Notice that I am only adding types to function parameters and returns, not their internal code.
This commit changes only
ProjectProxyso that we have an example. If we like it, I'll add type hints for the rest of the proxies as well.I don't use some of the modern type hinting features to remain compatible with EPEL8 and its Python 3.6, hence
List,Dict, andOptional.The reason behind this effort is the Copr MCP server https://github.com/fedora-copr/copr-mcp
It is supposed to produce the best results on typed functions.