Conversation
07pepa
requested review from
a team,
CAPITAINMARVEL,
LawMixer,
Riverfount,
adeelsohailahmed and
staticxterm
| - uses: actions/checkout@v2 | ||
| - uses: actions/setup-python@v2 | ||
| - uses: actions/checkout@v4 | ||
| - name: Set up Python ${{ inputs.python-version }} |
There was a problem hiding this comment.
Should be matrix instead, don't know why I put inputs in my review on the Beanie repo, probably I saw it in some tutorial so made an unintentional mistake...
| - name: Set up Python ${{ inputs.python-version }} | |
| - name: Set up Python ${{ matrix.python-version }} |
| python-version: ${{ matrix.python-version }} | ||
| - name: Run image | ||
| uses: abatilo/actions-poetry@v2 | ||
| python-version: ${{ inputs.python-version }} |
There was a problem hiding this comment.
Same as previous comment, otherwise the correct Python version wouldn't be picked up by the CI.
| python-version: ${{ inputs.python-version }} | |
| python-version: ${{ matrix.python-version }} |
.pre-commit-config.yaml
Outdated
| repos: | ||
| - repo: https://github.com/ambv/black | ||
| rev: 22.12.0 | ||
| - repo: https://github.com/charliermarsh/ruff-pre-commit |
There was a problem hiding this comment.
| - repo: https://github.com/charliermarsh/ruff-pre-commit | |
| - repo: https://github.com/astral-sh/ruff-pre-commit |
.pre-commit-config.yaml
Outdated
| - repo: https://github.com/ambv/black | ||
| rev: 22.12.0 | ||
| - repo: https://github.com/charliermarsh/ruff-pre-commit | ||
| rev: v0.11.8 |
There was a problem hiding this comment.
Current latest.
| rev: v0.11.8 | |
| rev: v0.11.10 |
| import pydantic_core | ||
| from pydantic import BaseModel, PrivateAttr, ValidationError, TypeAdapter | ||
| from pydantic import BaseModel, PrivateAttr, TypeAdapter, ValidationError | ||
| from pydantic._internal import _fields |
There was a problem hiding this comment.
I realize this comment is not related to the functionality this PR introduces but, I'm really struggling to see why do we need to import from Pydantic internal stuff.
The only method we use is on ln 76, the is_valid_field_name method and we could simply "vendor it in" ourselves as a helper method.
pyproject.toml
Outdated
| {name = "Roman Right", email = "roman-right@protonmail.com"}, | ||
| ] | ||
| license = {text = "Apache-2.0"} | ||
| requires-python = "<4.0,>=3.9" |
There was a problem hiding this comment.
I would revert this since it reads more naturally to me.
| requires-python = "<4.0,>=3.9" | |
| requires-python = ">=3.9,<4.0" |
pyproject.toml
Outdated
| "mypy>=1.15.0", | ||
| "pytest<8.0.0,>=7.2.0", | ||
| "pytest-cov>=6.1.1", | ||
| "pytest-sugar<2.0.0,>=1.0.0", |
There was a problem hiding this comment.
Where does this pytest-sugar dependency come from?
Since we don't use it on the Beanie repo I suggest that we don't use it here either.
| "pydantic>=2.10.0,<3.0", | ||
| ] | ||
| name = "lazy-model" | ||
| version = "0.3.0" |
There was a problem hiding this comment.
Seems like this project doesn't have the same setup as the Beanie project with regards to SemVer uplifts.
Therefore, it is needed to manually update the version specifier when commiting any change.
|
Generally it also looks good to me functionality wise, but the CI testing is broken and my comment for the Warning: Neither 'python-version' nor 'python-version-file' inputs were supplied. Attempting to find '.python-version' file.
Warning: .python-version doesn't exist.
Warning: The `python-version` input is not set. The version of Python currently in `PATH` will be used.It would always select and run the tests with the Python version available from the base image, you can verify this if you check the "run tests" step for each of the tests: Using CPython 3.12.3 interpreter at: /usr/bin/python3 |
07pepa
force-pushed
the
remove-pydanic1
branch
2 times, most recently
from
4fdd049 to
cae4426
Compare
4 participants
remove pydantic v1 and migrate to uv