Conversation
for more information, see https://pre-commit.ci
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2303 +/- ##
==========================================
- Coverage 82.22% 82.20% -0.03%
==========================================
Files 79 79
Lines 10077 10086 +9
Branches 1152 1154 +2
==========================================
+ Hits 8286 8291 +5
- Misses 1589 1592 +3
- Partials 202 203 +1 ☔ View full report in Codecov by Sentry. |
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Pull Request Overview
This PR updates the UUID generation utility to support UUID version 7 while retaining support for UUID4 as a fallback option. Key changes include switching the function parameter from a Callable to a version literal and introducing a try/except block for importing uuid7 with a fallback implementation.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
this will need tests update and thorough testing before merge |
|
My $0.02: it should still be possible to manually specify UUID4s - to not break the case where pre-generated UUID4s are passed to signatures. |
would you mind elaborate a bit more please? so that I can clearly understand what you wanted to be done here? |
Sure: This use case should not break: |
|
OK, thanks a lot! |
Co-authored-by: Gert Van Gool <gertvangool@gmail.com>
|
Found 1 test failure on Blacksmith runners: Failure
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (3)
kombu/utils/uuid.py:23
- There are blank lines in the function docstring that contain trailing whitespace (between sections). Flake8 will flag these as
W293 blank line contains whitespace; please remove the spaces on the blank lines in the docstring.
"""Generate a unique ID based on the specified UUID version (4 or 7).
Parameters
----------
version : Literal[4, 7]
The UUID version to use for generating the unique ID. Defaults to 7.
If UUID7 is unavailable, it falls back to UUID4 regardless of the input.
kombu/utils/uuid.py:8
- The comment "Python 3.14 or later" is likely to become misleading (this project currently declares support up to Python 3.13). Suggest making the comment version-agnostic (e.g., "Python versions that include uuid7") or removing it.
# Python 3.14 or later
from uuid import uuid7
kombu/utils/uuid.py:19
- New branching behavior (
versionselection,ValueErroron invalid input, and UUID7 fallback) is introduced here, but the existingt/unit/utils/test_uuid.pytests only coveruuid()returning unique strings. Please extend tests to coveruuid(version=4),uuid(version=7), and the invalid-version error path so regressions are caught.
def uuid(version: Literal[4, 7] = 7) -> str:
"""Generate a unique ID based on the specified UUID version (4 or 7).
Parameters
----------
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def uuid(version: Literal[4, 7] = 7) -> str: | ||
| """Generate a unique ID based on the specified UUID version (4 or 7). |
There was a problem hiding this comment.
This changes the public uuid() API from accepting an arbitrary UUID factory callable to accepting a version integer. Since uuid is re-exported from kombu.common (and likely used externally), this is a backward-incompatible signature change. Consider preserving the previous callable-based parameter for compatibility (e.g., keep a _uuid: Callable[...] | None positional arg and make version keyword-only), or introduce a new function for UUID7 while keeping the old behavior.
| from __future__ import annotations | ||
|
|
||
| from typing import Callable | ||
| from uuid import UUID, uuid4 | ||
| from typing import Literal | ||
| from uuid import uuid4 |
There was a problem hiding this comment.
The module-level docstring was removed, and this repo appears to require module docstrings (flake8-docstrings would report D100 for a public module). Please re-add the "UUID utilities." docstring at the top of the file (before the __future__ import).
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
No description provided.