Skip to content
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.

Sign up for GitHub

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

Jump to bottom

chore(deps): remove email-validator; use Str over EmailStr in SES model #1608

Merged
merged 5 commits into from
Oct 18, 2022
Merged

chore(deps): remove email-validator; use Str over EmailStr in SES model #1608

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
32d5d05
chore: remove email-validator dependency from the all group
rubenfonseca Oct 14, 2022
f815d31
fix: add dev poetry group to install all dev dependencies
rubenfonseca Oct 14, 2022
d63694d
Revert "fix: add dev poetry group to install all dev dependencies"
rubenfonseca Oct 17, 2022
69814e2
fix: remove email-validator dependency
rubenfonseca Oct 17, 2022
6507bee
fix: remove references to EmailStr
rubenfonseca Oct 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 4 additions & 5 deletions aws_lambda_powertools/utilities/parser/models/ses.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
from typing import List, Optional

from pydantic import BaseModel, Field
from pydantic.networks import EmailStr
from pydantic.types import PositiveInt

from ..types import Literal
Expand All @@ -21,7 +20,7 @@ class SesReceiptAction(BaseModel):
class SesReceipt(BaseModel):
timestamp: datetime
processingTimeMillis: PositiveInt
recipients: List[EmailStr]
recipients: List[str]
Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @rubenfonseca @heitorlessa , that's a breaking change. users are expecting here proper email address validation.
I think a better solution would be:

  1. During import try to import EmailStr, if there's an except redefine the type as str
  2. add docs and specify that for Ses model, you should install the extra dependency

WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @ran-isenberg , we hear you and appreciate your feedback. We’re including this change in V2 to not impact customers in any shape or form.

To add to my comment earlier, as helpful as e-mail validator may be for an untrusted payload, this is not the case for SES. It would, however, for something like “body” in API Gateway.

I’ll update the issue linked with this and related Pydantic overhead that are making us more cautious on double validation.

spamVerdict: SesReceiptVerdict
virusVerdict: SesReceiptVerdict
spfVerdict: SesReceiptVerdict
Expand All @@ -41,17 +40,17 @@ class SesMailCommonHeaders(BaseModel):
bcc: Optional[List[str]]
sender: Optional[List[str]]
reply_to: Optional[List[str]] = Field(None, alias="reply-to")
returnPath: EmailStr
returnPath: str
messageId: str
date: str
subject: str


class SesMail(BaseModel):
timestamp: datetime
source: EmailStr
source: str
messageId: str
destination: List[EmailStr]
destination: List[str]
headersTruncated: bool
headers: List[SesMailHeaders]
commonHeaders: SesMailCommonHeaders
Expand Down
6 changes: 3 additions & 3 deletions layer/layer/canary/app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from importlib.metadata import version

import boto3
from pydantic import EmailStr
from pydantic import HttpUrl

from aws_lambda_powertools import Logger, Metrics, Tracer
from aws_lambda_powertools.utilities.parser import BaseModel, envelopes, event_parser
Expand All @@ -22,12 +22,12 @@
event_bus_arn = os.getenv("VERSION_TRACKING_EVENT_BUS_ARN")


# Model to check parser imports correctly, tests for pydantic and email-validator
# Model to check parser imports correctly, tests for pydantic
class OrderItem(BaseModel):
order_id: int
quantity: int
description: str
email: EmailStr
url: HttpUrl


# Tests for jmespath presence
Expand Down
64 changes: 15 additions & 49 deletions poetry.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 2 additions & 3 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ python = "^3.7.4"
aws-xray-sdk = { version = "^2.8.0", optional = true }
fastjsonschema = { version = "^2.14.5", optional = true }
pydantic = { version = "^1.8.2", optional = true }
email-validator = { version = "^1.3.0", optional = true }
boto3 = { version = "^1.20.32", optional = true }

[tool.poetry.dev-dependencies]
Expand Down Expand Up @@ -77,10 +76,10 @@ mypy-boto3-appconfigdata = "^1.24.36"
importlib-metadata = "^4.13"

[tool.poetry.extras]
parser = ["pydantic", "email-validator"]
parser = ["pydantic"]
validation = ["fastjsonschema"]
tracer = ["aws-xray-sdk"]
all = ["pydantic", "email-validator", "aws-xray-sdk", "fastjsonschema"]
all = ["pydantic", "aws-xray-sdk", "fastjsonschema"]
# allow customers to run code locally without emulators (SAM CLI, etc.)
aws-sdk = ["boto3"]
[tool.coverage.run]
Expand Down
3 changes: 1 addition & 2 deletions tox.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,9 @@ deps =
filelock
pytest-xdist
pydantic
email-validator

commands = python parallel_run_e2e.py

; If you ever encounter another parallel lock across interpreters
; pip install tox tox-poetry
; tox -p --parallel-live
; tox -p --parallel-live