Skip to content

Commit

Permalink
Merge pull request #204 from lsst-sqre/tickets/DM-40744
Browse files Browse the repository at this point in the history
DM-40744: Convert to Pydantic v2
  • Loading branch information
rra authored Sep 18, 2023
2 parents 9134a10 + 65fb84d commit 2a3e095
Show file tree
Hide file tree
Showing 31 changed files with 285 additions and 185 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,6 @@ init:
# level of shell trickery after failed commands.
.PHONY: linkcheck
linkcheck:
sphinx-build --keep-going -n -W -T -b linkcheck docs \
sphinx-build --keep-going -n -T -b linkcheck docs \
docs/_build/linkcheck \
|| (cat docs/_build/linkcheck/output.txt; exit 1)
4 changes: 4 additions & 0 deletions changelog.d/20230911_180355_rra_DM_40744.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
### Backwards-incompatible changes

- Safir now depends on Pydantic v2. Python code that uses any part of Safir related to Pydantic will also need to update to Pydantic v2, since the API is significantly different. See the [Pydantic migration guide](https://docs.pydantic.dev/latest/migration/) for more information.
- `safir.pydantic.validate_exactly_one_of` is now a Pydantic model validator. It must be called with `mode="after"`, since it operates in the model rather than on a raw dictionary.
3 changes: 3 additions & 0 deletions changelog.d/20230914_153856_rra_DM_40744.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
### Bug fixes

- `safir.database.datetime_to_db`, `safir.datetime.format_datetime_for_logging`, and `safir.datetime.isodatetime` now accept any `datetime` object with a time zone whose offset from UTC is 0, rather than only the `datetime.UTC` time zone object.
3 changes: 3 additions & 0 deletions changelog.d/20230918_114322_rra_DM_40744.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
### Backwards-incompatible changes

- `safir.github.GitHubAppClientFactory` now expects the application ID and installation ID (for `create_installation_client`) to be of type `int`, not `str`. This appears to match what GitHub's API returns, but not what Gidgethub expects. The ID is converted to a string when passing it to Gidgethub.
3 changes: 3 additions & 0 deletions changelog.d/20230918_134911_rra_DM_40744.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
### Bug fixes

- `safir.pydantic.normalize_datetime` now explicitly rejects input other than seconds since epoch or datetime objects with a validation error rather than attempting to treat the input as a datetime object and potentially throwing more obscure errors.
9 changes: 6 additions & 3 deletions docs/user-guide/arq.rst
Original file line number Diff line number Diff line change
Expand Up @@ -45,16 +45,19 @@ If your app uses a configuration system like ``pydantic.BaseSettings``, this exa
from urllib.parse import urlparse
from arq.connections import RedisSettings
from pydantic import BaseSettings, Field, RedisDsn
from pydantic import Field, RedisDsn
from pydantic_settings import BaseSettings
from safir.arq import ArqMode
class Config(BaseSettings):
arq_queue_url: RedisDsn = Field(
"redis://localhost:6379/1", env="APP_ARQ_QUEUE_URL"
"redis://localhost:6379/1", validation_alias="APP_ARQ_QUEUE_URL"
)
arq_mode: ArqMode = Field(ArqMode.production, env="APP_ARQ_MODE")
arq_mode: ArqMode = Field(
ArqMode.production, validation_alias="APP_ARQ_MODE"
)
@property
def arq_redis_settings(self) -> RedisSettings:
Expand Down
5 changes: 2 additions & 3 deletions docs/user-guide/datetime.rst
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,14 @@ To use this format as the serialized representation of any `~datetime.datetime`
from datetime import datetime
from pydantic import BaseModel
from pydantic import BaseModel, field_serializer
from safir.datetime import isodatetime
class Example(BaseModel):
some_time: datetime
class Config:
json_encoders = {datetime: lambda v: isodatetime(v)}
_serialize_some_time = field_serializer("some_time")(isodatetime)
Also see the Pydantic validation function `safir.pydantic.normalize_isodatetime`, discussed further at :ref:`pydantic-datetime`.

Expand Down
2 changes: 1 addition & 1 deletion docs/user-guide/fastapi-errors.rst
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ The code to raise ``fastapi.HTTPException`` should therefore look something like
)
raise HTTPException(
status_code=status.HTTP_404_NOT_FOUND,
detail=[error.dict(exclude_none=True)],
detail=[error.model_dump(exclude_none=True)],
)
Declaring the error model
Expand Down
13 changes: 9 additions & 4 deletions docs/user-guide/github-apps/create-a-github-client.rst
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ For information about creating a GitHub App, retrieving its App ID and generatin

.. code-block:: python
from pydantic import BaseSettings, Field, SecretStr
from pydantic import Field, SecretStr
from pydantic_settings import BaseSettings
from safir.github import GitHubAppClientFactory
Expand All @@ -41,11 +42,15 @@ For information about creating a GitHub App, retrieving its App ID and generatin
GitHub App functionality.
"""
github_app_id: str = Field(env="GITHUB_APP_ID")
github_app_id: str = Field(validation_alias="GITHUB_APP_ID")
github_webhook_secret: SecretStr = Field(env="GITHUB_WEBHOOK_SECRET")
github_webhook_secret: SecretStr = Field(
validation_alias="GITHUB_WEBHOOK_SECRET"
)
github_app_private_key: SecretStr = Field(env="GITHUB_APP_PRIVATE_KEY")
github_app_private_key: SecretStr = Field(
validation_alias="GITHUB_APP_PRIVATE_KEY"
)
config = Config()
Expand Down
7 changes: 4 additions & 3 deletions docs/user-guide/github-apps/handling-webhooks.rst
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,9 @@ The URL path for this endpoint corresponds to the webhook callback URL you set u
body = await request.body()
event = Event.from_http(request.headers, body, secret=webhook_secret)
# Bind the X-GitHub-Delivery header to the logger context; this identifies
# the webhook request in GitHub's API and UI for diagnostics
# Bind the X-GitHub-Delivery header to the logger context; this
# identifies the webhook request in GitHub's API and UI for
# diagnostics
logger = logger.bind(github_delivery=event.delivery_id)
logger.debug("Received GitHub webhook", payload=event.data)
Expand Down Expand Up @@ -138,7 +139,7 @@ You can parse the ``event.data`` attribute into a Pydantic model using the ``par
f"Received {event.event} {event.data.action} event",
event=event.event,
action=event.data.action,
payload=pull_request_event.dict(),
payload=pull_request_event.model_dump(),
number=pull_request_event.number,
)
Expand Down
41 changes: 26 additions & 15 deletions docs/user-guide/pydantic.rst
Original file line number Diff line number Diff line change
Expand Up @@ -13,33 +13,37 @@ Normalizing datetime fields
Pydantic supports several input formats for `~datetime.datetime` fields, but the resulting `~datetime.datetime` object may be timezone-naive.
Best practice for Python code is to only use timezone-aware `~datetime.datetime` objects in the UTC time zone.

Pydantic provides a utility function, `~safir.pydantic.normalize_datetime`, that can be used as a validator for a `~datetime.datetime` model field.
Pydantic provides a utility function, `~safir.pydantic.normalize_datetime`, that can be used as a field validator for a `~datetime.datetime` model field.
It ensures that any input is converted to UTC and is always timezone-aware.

Here's an example of how to use it:

.. code-block:: python
from pydantic import BaseModel, field_validator
from safir.pydantic import normalize_datetime
class Info(BaseModel):
last_used: Optional[datetime] = Field(
None,
title="Last used",
description="When last used in seconds since epoch",
example=1614986130,
examples=[1614986130],
)
_normalize_last_used = validator(
"last_used", allow_reuse=True, pre=True
)(normalize_datetime)
_normalize_last_used = field_validator("last_used", mode="before")(
normalize_datetime
)
Multiple attributes can be listed as the initial arguments of `~pydantic.validator` if there are multiple fields that need to be checked.
Multiple attributes can be listed as the initial arguments of `~pydantic.field_validator` if there are multiple fields that need to be checked.

This validator accepts all of the input formats that Pydantic accepts.
This field validator accepts all of the input formats that Pydantic accepts.
This includes some ambiguous formats, such as an ISO 8601 date without time zone information.
All such dates are given a consistent interpretation as UTC, but the results may be surprising if the caller expected local time.
In some cases, it may be desirable to restrict input to one unambiguous format.

This can be done by using `~safir.pydantic.normalize_isodatetime` as the validator instead.
This can be done by using `~safir.pydantic.normalize_isodatetime` as the field validator instead.
This function only accepts ``YYYY-MM-DDTHH:MM[:SS]Z`` as the input format.
The ``Z`` time zone prefix indicating UTC is mandatory.
It is called the same way as `~safir.pydantic.normalize_datetime`.
Expand All @@ -56,19 +60,23 @@ To use it, add a configuration block to any Pydantic model that has snake-case a

.. code-block:: python
from pydantic import BaseModel, ConfigDict
from safir.pydantic import to_camel_case
class Model(BaseModel):
some_field: str
class Config:
alias_generator = to_camel_case
allow_population_by_field_name = True
model_config = ConfigDict(
alias_generator=to_camel_case, populate_by_name=True
)
By default, only the generated aliases (so, in this case, only the camel-case form of the attribute, ``someField``) are supported.
The additional setting ``allow_population_by_field_name``, tells Pydantic to allow either ``some_field`` or ``someField`` in the input.

As a convenience, you can instead inherit from `~safir.pydantic.CamelCaseModel`, which is a derived class of `~pydantic.BaseModel` with those settings added.
This is somewhat less obvious when reading the classes and thus less self-documenting, but is less tedious if you have numerous models that need to support camel-case.
`~safir.pydantic.CamelCaseModel` also overrides ``dict`` and ``json`` to change the default of ``by_alias`` to `True` so that this model exports in camel-case by default.
`~safir.pydantic.CamelCaseModel` also overrides ``model_dump`` and ``model_dump_json`` to change the default of ``by_alias`` to `True` so that this model exports in camel-case by default.

Requiring exactly one of a list of attributes
=============================================
Expand All @@ -86,19 +94,22 @@ The intent here is that only one of those two configurations will be present: ei
However, Pydantic has no native way to express that, and the above model will accept input where neither or both of those attributes are set.

Safir provides a function, `~safir.pydantic.validate_exactly_one_of`, designed for this case.
It takes a list of fields, of which exactly one must be set, and builds a root validator function that checks this property of the model.
It takes a list of fields, of which exactly one must be set, and builds a model validator function that checks this property of the model.

So, in the above example, the full class would be:

.. code-block:: python
from pydantic import BaseModel, model_validator
from safir.pydantic import validate_exactly_one_of
class Model(BaseModel):
docker: Optional[DockerConfig] = None
ghcr: Optional[GHCRConfig] = None
_validate_type = root_validator(allow_reuse=True)(
_validate_type = model_validator(mode="after")(
validate_exactly_one_of("docker", "ghcr")
)
Note the syntax, which is a little odd since it is calling a decorator on the results of a function builder.
``allow_reuse=True`` must be set due to limitations in Pydantic.
10 changes: 2 additions & 8 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ dependencies = [
"fastapi<1",
"gidgethub<6",
"httpx>=0.20.0,<1",
"pydantic<2",
"pydantic>2,<3",
"starlette<1",
"structlog>=21.2.0,<24",
]
Expand Down Expand Up @@ -56,7 +56,7 @@ dev = [
"types-redis",
"uvicorn",
# documentation
"documenteer[guide]>=0.7.0,<1",
"documenteer[guide]>=1.0.0a7",
"autodoc_pydantic",
]
gcs = [
Expand Down Expand Up @@ -265,12 +265,6 @@ builtins-ignorelist = [
fixture-parentheses = false
mark-parentheses = false

[tool.ruff.pep8-naming]
classmethod-decorators = [
"pydantic.root_validator",
"pydantic.validator",
]

[tool.ruff.pydocstyle]
convention = "numpy"

Expand Down
9 changes: 7 additions & 2 deletions src/safir/asyncio.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,11 @@ def aiter_from(
timeout waiting for the next item; this is the total execution
time of the iterator.
Returns
-------
AsyncIterator
An async iterator over the contents of the queue.
Raises
------
TimeoutError
Expand Down Expand Up @@ -176,8 +181,8 @@ def put(self, item: T) -> None:
Raises
------
AsyncMultiQueueError
Raised if `put` was called after `end` without an intervening call
to `clear`.
Raised if `put` was called after `close` without an intervening
call to `clear`.
"""
if self.finished:
msg = "end was already called, must call clear before put"
Expand Down
4 changes: 2 additions & 2 deletions src/safir/database.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

import asyncio
import time
from datetime import UTC, datetime
from datetime import UTC, datetime, timedelta
from typing import overload
from urllib.parse import quote, urlparse

Expand Down Expand Up @@ -140,7 +140,7 @@ def datetime_to_db(time: datetime | None) -> datetime | None:
"""
if not time:
return None
if time.tzinfo != UTC:
if time.utcoffset() != timedelta(seconds=0):
raise ValueError(f"datetime {time} not in UTC")
return time.replace(tzinfo=None)

Expand Down
6 changes: 3 additions & 3 deletions src/safir/datetime.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

from __future__ import annotations

from datetime import UTC, datetime
from datetime import UTC, datetime, timedelta
from typing import overload

__all__ = [
Expand Down Expand Up @@ -75,7 +75,7 @@ def format_datetime_for_logging(timestamp: datetime | None) -> str | None:
Raised if the argument is in a time zone other than UTC.
"""
if timestamp:
if timestamp.tzinfo not in (None, UTC):
if timestamp.utcoffset() != timedelta(seconds=0):
raise ValueError(f"datetime {timestamp} not in UTC")
if timestamp.microsecond:
result = timestamp.isoformat(sep=" ", timespec="milliseconds")
Expand Down Expand Up @@ -106,7 +106,7 @@ def isodatetime(timestamp: datetime) -> str:
ValueError
The provided timestamp was not in UTC.
"""
if timestamp.tzinfo not in (None, UTC):
if timestamp.utcoffset() != timedelta(seconds=0):
raise ValueError(f"datetime {timestamp} not in UTC")
return timestamp.strftime("%Y-%m-%dT%H:%M:%SZ")

Expand Down
16 changes: 11 additions & 5 deletions src/safir/github/_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,16 @@ class GitHubAppClientFactory:
from (e.g. ``lsst-sqre/times-square``).
http_client
The httpx client.
Notes
-----
Gidgethub treats the application ID and installation ID as strings, but
GitHub's API appears to return them as integers. This class expects them
to be integers and converts them to strings when calling Gidgethub.
"""

def __init__(
self, *, id: str, key: str, name: str, http_client: httpx.AsyncClient
self, *, id: int, key: str, name: str, http_client: httpx.AsyncClient
) -> None:
self.app_id = id
self.app_key = key
Expand All @@ -43,7 +49,7 @@ def get_app_jwt(self) -> str:
The JWT token.
"""
return gidgethub.apps.get_jwt(
app_id=self.app_id, private_key=self.app_key
app_id=str(self.app_id), private_key=self.app_key
)

def _create_client(self, *, oauth_token: str | None = None) -> GitHubAPI:
Expand All @@ -62,7 +68,7 @@ def create_anonymous_client(self) -> GitHubAPI:
return self._create_client()

async def create_installation_client(
self, installation_id: str
self, installation_id: int
) -> GitHubAPI:
"""Create a client authenticated as an installation of the GitHub App
for a specific repository or organization.
Expand All @@ -83,8 +89,8 @@ async def create_installation_client(
anon_client = self.create_anonymous_client()
token_info = await gidgethub.apps.get_installation_access_token(
anon_client,
installation_id=installation_id,
app_id=self.app_id,
installation_id=str(installation_id),
app_id=str(self.app_id),
private_key=self.app_key,
)
return self._create_client(oauth_token=token_info["token"])
Expand Down
Loading

0 comments on commit 2a3e095

Please sign in to comment.