diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml index 21a8f3b035c..06646e0d4f7 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.yml +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -45,6 +45,16 @@ body: description: Please share how we might be able to reproduce this issue validations: required: true + - type: dropdown + id: reproduced + attributes: + label: Did you run the reproduction yourself? + description: We welcome reports written with AI assistance; tell us whether the snippet above was executed and the failure observed, or derived from reading the code. + options: + - Yes, I ran it and saw the failure + - No, this comes from reading the code + validations: + required: true - type: input id: version attributes: diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 234a347f24a..659483b7032 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -11,6 +11,11 @@ > Please share what the user experience looks like before and after this change +### Verification + +> How did you verify this change? Tests you ran yourself, and the real AWS service or event you validated against when it applies. +> If you used an AI assistant to write any part of this PR, say so here. That is welcome; what we ask is the verification above, done by you. + +# Working on Powertools for AWS Lambda (Python) + +Short guide for anyone, human or coding agent, changing this repository. It points to the sources of truth instead of repeating them. Read [CONTRIBUTING.md](CONTRIBUTING.md) for the process and [docs/adr](docs/adr/README.md) for the premises behind the rules below. + +## Before you start + +- Work from an issue that a maintainer triaged. PRs on untriaged issues usually wait or get closed. +- One issue, one PR. Do not bundle unrelated fixes, even small ones. +- Branch from `develop`. Keep the diff limited to what the issue needs; do not reformat or "improve" code you did not have to touch. + +## Commands + +```bash +make dev # uv sync, pre-commit hooks +make lint # ruff format + ruff check (aws_lambda_powertools, tests, examples) +make mypy # static typing, must pass +uv run pytest tests/unit tests/functional/ # fast loop for one utility +make test # full unit + functional + performance, what CI runs +make pr # everything CI checks, run it before opening the PR +``` + +Do not run the full suite with `pytest -n auto`; collection differs between workers and it fails for reasons unrelated to your change. + +## What a PR needs to pass + +- Title follows [conventional commits](.github/semantic.yml): `fix(parameters): ...`, `feat(event_handler): ...`, `docs(...)`. Scope is the utility name. +- Description contains `closes #` alone on its own line. Trailing text on that line breaks the related-issue check. +- Tests for the change. A bug fix adds a test that fails on `develop` and passes with the fix. +- Functional tests use the public API only, the way a customer would call it. Never test internal methods: do not call or patch private methods, do not assert on private attributes. +- Existing tests are not modified or deleted unless the PR proves the old behaviour was a bug. Say so in the description. +- `ruff`, `mypy` and the test suite green on Python 3.10 to 3.14. CI runs all five. +- Behaviour changes update the docs page and its example under `examples//src/`. + +## Things we do not do + +- Add a runtime dependency. Optional features go behind an extra in `pyproject.toml`; the core stays `jmespath` and `typing-extensions`. See [ADR 0007](docs/adr/0007-lean-dependencies.md). +- Import heavy modules at module level. Cold start matters; import lazily inside the function or utility that needs it. See [ADR 0002](docs/adr/0002-every-millisecond-counts.md). +- Ship `boto3`, `botocore` or their dependencies in the Lambda layer. See [ADR 0004](docs/adr/0004-layer-and-packaging.md). +- Break or change public behaviour in a minor release. If an input we accept today is wrong, warn with `PowertoolsUserWarning` now and raise in the next major. See [ADR 0006](docs/adr/0006-zero-regression-and-compatibility.md). +- Duplicate a code path to add a variant (async, a new event shape). Share the logic and keep the difference to the call site. +- Change `tests/events/*.json` fixtures in a way that hides a bug. Add a new fixture instead. +- Use threads, background work or anything that assumes the process outlives the invocation. See [ADR 0001](docs/adr/0001-lambda-first.md). + +## Verifying your change + +- Run the tests yourself and read the output. Do not describe runs you did not do. +- If the change touches an AWS service integration (Parameters, Idempotency, Batch, Data Classes shapes, Event Handler payloads), validate it against the real service or a real event and say in the PR how you did it. Mocks prove the logic, not the contract. +- If you used an AI assistant, say so in the PR. We welcome it. What we ask is the verification above, done by you. + +## Where things are + +| Need | Look at | +| --- | --- | +| Utility code | `aws_lambda_powertools//` or `aws_lambda_powertools/utilities//` | +| Tests | `tests/unit//`, `tests/functional//`, `tests/performance/` | +| Sample events | `tests/events/` | +| Docs and examples | `docs//.md`, `examples//src/` | +| Project premises | `docs/adr/` | +| Layer build | `layer_v3/` | +| Maintainer process | [MAINTAINERS.md](MAINTAINERS.md), [docs/maintainers.md](docs/maintainers.md) | diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 00000000000..43c994c2d36 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1 @@ +@AGENTS.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5402c73fa7c..f03a04f06ff 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -4,6 +4,7 @@ - [Contributing Guidelines](#contributing-guidelines) - [Reporting Bugs/Feature Requests](#reporting-bugsfeature-requests) - [Contributing via Pull Requests](#contributing-via-pull-requests) + - [AI-assisted contributions](#ai-assisted-contributions) - [Dev setup](#dev-setup) - [Local documentation](#local-documentation) - [Conventions](#conventions) @@ -96,6 +97,15 @@ timeline : Update draft release ``` +### AI-assisted contributions + +Contributions written with the help of an AI assistant are welcome, under two conditions: + +1. Say so in the issue or the pull request. There is a field for it in both templates. +2. Verify the change yourself and describe how. Run the tests and read the output; when the change touches an AWS service integration, validate it against the real service or a real event and tell us what you did. Mocks prove the logic, not the contract. + +The same rules apply to everyone: one issue per PR, tests through the public API, no changes to existing tests without proof the old expectation was a bug. [AGENTS.md](AGENTS.md) lists them with the commands to run, and [docs/adr](docs/adr/README.md) explains why. + ### Dev setup Firstly, [fork the repository](https://github.com/aws-powertools/powertools-lambda-python/fork). diff --git a/README.md b/README.md index 34d3cac965c..f8298a6bea1 100644 --- a/README.md +++ b/README.md @@ -94,6 +94,10 @@ Share what you did with Powertools for AWS Lambda (Python) 💞💞. Blog post, This helps us understand who uses Powertools for AWS Lambda (Python) in a non-intrusive way, and helps us gain future investments for other Powertools for AWS Lambda languages. When [using Layers](https://docs.powertools.aws.dev/lambda/python/latest/#lambda-layer), you can add Powertools for AWS Lambda (Python) as a dev dependency (or as part of your virtual env) to not impact the development process. +## Contributing + +Issues and pull requests are welcome, including the ones written with AI assistance. If you used an assistant, say so in the issue or PR and tell us how you verified the change: which tests you ran yourself, and which real AWS service or event you validated against when it applies. Read [CONTRIBUTING.md](CONTRIBUTING.md) for the process and [AGENTS.md](AGENTS.md) for the rules the CI enforces and the premises behind them. + ## Credits * Structured logging initial implementation from [aws-lambda-logging](https://gitlab.com/hadrien/aws_lambda_logging) diff --git a/docs/adr/0001-lambda-first.md b/docs/adr/0001-lambda-first.md new file mode 100644 index 00000000000..0cfbcefe5ce --- /dev/null +++ b/docs/adr/0001-lambda-first.md @@ -0,0 +1,32 @@ + +# 0001. Lambda first + +Status: Accepted + +## Context + +Powertools for AWS Lambda (Python) runs inside AWS Lambda. That execution model is specific: the service decides when an execution environment is created, reused, paused or recycled, how many invocations it handles and whether they run concurrently (as with Lambda Managed Instances), `/tmp` is the only writable disk, and any call may be a fresh cold start. + +Code that works well in a long-running web server (background threads, connection pools warmed at import, periodic refresh timers, in-memory state that assumes to survive) behaves differently or breaks here. + +The utilities also get used outside Lambda, in local development, tests, containers and sometimes in other frameworks. That usage is welcome but it is not what we design for. + +## Decision + +We design, test and optimise for the Lambda execution model only. + +- A utility must be correct when the environment is paused right after the handler returns, when it never runs again, and when several invocations run in it at once. +- No background threads, timers or work that assumes the process stays alive after the invocation. Anything that must happen, happens inside the invocation. +- In-memory state (caches, counters, connections) is treated as per execution environment: it may be reused across invocations, it may vanish at any time, and it may be touched by concurrent invocations. Anything shared must be safe under that concurrency. +- Cold start is a first-class scenario. See [ADR 0002](0002-every-millisecond-counts.md). +- Behaviour outside Lambda is best effort. We do not add code paths for other runtimes, and we do not accept changes whose only benefit shows outside Lambda. + +## Consequences + +- Reviews ask "what happens on a cold start, and what happens if the environment is recycled right after this?" for every stateful change. +- Concurrency inside one environment comes from concurrent invocations, from the customer's own threads and from coroutines, so thread and coroutine safety is a requirement, not a nicety (see the circuit breaker's single-prober guarantee). Coordination across environments goes through AWS services, never shared memory. +- Features that need a long-lived process are declined or redesigned around an invocation boundary. + +## When reviewing or writing a change + +Ask where the state lives and who cleans it up. If the answer depends on the process surviving, it does not fit. diff --git a/docs/adr/0002-every-millisecond-counts.md b/docs/adr/0002-every-millisecond-counts.md new file mode 100644 index 00000000000..a89a41decf4 --- /dev/null +++ b/docs/adr/0002-every-millisecond-counts.md @@ -0,0 +1,29 @@ + +# 0002. Every millisecond counts + +Status: Accepted + +## Context + +Powertools for AWS Lambda (Python) is imported at the top of most handlers that use it, so our import time is part of every cold start of every customer. A dependency pulled in at import, a schema compiled at module load or a client created eagerly costs milliseconds on each cold start for everyone, including customers who never use that feature. + +We measure this. `tests/performance/` has import-time and initialisation tests with hard limits, and they fail the build when crossed. The layer ships compiled bytecode for the same reason ([ADR 0004](0004-layer-and-packaging.md)). + +## Decision + +Startup cost is a feature we protect. + +- Importing `aws_lambda_powertools` and any single utility must stay within the limits in `tests/performance/`. A change that moves a limit has to justify it in the PR, and the justification is the exception, not the norm. +- Optional and heavy dependencies (`pydantic`, `aws-xray-sdk`, `fastjsonschema`, `redis`, `boto3` itself) are imported only by the utility that uses them, never by the package root or by a shared module. A customer who imports the Logger must not pay for `pydantic`; a customer who imports the Parser must not pay for `aws-xray-sdk`. +- Nothing runs at import time beyond defining names. No network calls, no client creation, no file reads, no schema compilation. Work happens on first use. +- New code paths in the hot path of an invocation (per record, per request) are reviewed for allocations and repeated work the same way. + +## Consequences + +- Some code is less tidy than it could be: imports inside functions, `TYPE_CHECKING` blocks for type-only imports, lazy loaders for optional modules. That is intentional. +- Adding a dependency is a performance decision as much as a maintenance one ([ADR 0007](0007-lean-dependencies.md)). +- Performance tests can flake on shared CI runners. We re-run them; we do not raise the limits to make them pass. + +## When reviewing or writing a change + +Look at the top of every touched module. If a new import is not from the standard library or from `aws_lambda_powertools` core, ask why it is not lazy. Run `tests/performance/` locally when you touch import paths. diff --git a/docs/adr/0003-python-versions.md b/docs/adr/0003-python-versions.md new file mode 100644 index 00000000000..038a1dd135c --- /dev/null +++ b/docs/adr/0003-python-versions.md @@ -0,0 +1,28 @@ + +# 0003. Python versions follow the Lambda runtimes + +Status: Accepted + +## Context + +Lambda adds a Python runtime shortly after each CPython release and deprecates runtimes on its own schedule. Customers move at different speeds, and a library that drops a version early strands people who cannot upgrade yet, while a library that lags blocks early adopters from using the newest runtime. + +## Decision + +Our supported Python versions are exactly the Python runtimes Lambda supports. + +- We add a version when the Lambda runtime becomes available, and ideally test against pre-release builds so support lands on day one. +- We drop a version only after Lambda deprecates its runtime, following the Lambda runtime deprecation dates. +- The CI matrix (`quality_check`) runs the full suite on every supported version. Green on one version is not green. +- Syntax and standard-library usage are limited to the oldest supported version. Newer language features arrive when the oldest runtime that lacks them is dropped, never before. +- The Lambda layer is published for every supported version and both architectures ([ADR 0004](0004-layer-and-packaging.md)). + +## Consequences + +- `requires-python` in `pyproject.toml` and the CI matrix move together, in a dedicated maintenance PR, not as a side effect of a feature. +- Code that needs a newer feature carries a compatibility path or waits. +- Dependencies we accept must themselves support the whole range ([ADR 0007](0007-lean-dependencies.md)). + +## When reviewing or writing a change + +Check the oldest version in the matrix before using a new syntax or stdlib API. If CI only ran on the latest version locally, run at least the oldest one too. diff --git a/docs/adr/0004-layer-and-packaging.md b/docs/adr/0004-layer-and-packaging.md new file mode 100644 index 00000000000..f3178f0008e --- /dev/null +++ b/docs/adr/0004-layer-and-packaging.md @@ -0,0 +1,28 @@ + +# 0004. Layer and packaging + +Status: Accepted + +## Context + +We have customers consuming Powertools for AWS Lambda (Python) through the public Lambda layer, built in `layer_v3/`, and customers packaging it themselves in a zip or container image. Both are first-class. The layer sits on `sys.path` ahead of the Lambda runtime's own packages, so anything we ship in it shadows what the runtime provides. + +The runtime already provides `boto3`, `botocore` and their dependencies (`urllib3`, `s3transfer`, `python-dateutil`, `six`, `jmespath`). When we shipped `urllib3` by accident (see issues 8555 and 8556), customers got a version of it they did not choose and that could differ from the SDK the runtime expects. + +## Decision + +- The layer never ships `boto3`, `botocore` or any of their dependencies. The runtime provides them, or the customer packages the version they want. Extras that pull them in are not installed into the layer. +- The layer is built from the released package with the same build for every supported Python version and for `x86_64` and `arm64`. The two architectures must be functionally identical. +- The layer ships one compiled `.pyc` per `.py` (`compileall` with checked hashes) so cold starts do not pay for bytecode compilation. +- The layer contains Powertools for AWS Lambda (Python) and its optional extras only. A dependency we would not accept in `pyproject.toml` does not enter the layer either ([ADR 0007](0007-lean-dependencies.md)). +- The zip package built from the wheel is the reference for customers who do not use the layer; what works in the layer must work in the zip and vice versa. + +## Consequences + +- Before a release the layer is built per version and architecture and exercised in real functions, both as a layer and as a zip, on cold and warm starts. +- A change in `pyproject.toml` extras or dependencies is also a layer change and is reviewed as one. +- Customers who need a specific `boto3` version keep full control of it. + +## When reviewing or writing a change + +If a PR touches dependencies, extras or `layer_v3/`, check what ends up in `python/lib/python3.*/site-packages` of a built layer. Nothing from the runtime's own set may appear there. diff --git a/docs/adr/0005-tests-are-a-contract.md b/docs/adr/0005-tests-are-a-contract.md new file mode 100644 index 00000000000..57f7e84be7c --- /dev/null +++ b/docs/adr/0005-tests-are-a-contract.md @@ -0,0 +1,30 @@ + +# 0005. Tests are a contract + +Status: Accepted + +## Context + +The test suite encodes the behaviour customers rely on. When a change makes an existing test fail, there are two possibilities: the change broke a contract, or the test was pinning a bug. Editing the test to make it pass hides which one it was. We have seen fixes land where a test expectation was flipped without evidence, and the "fix" was in fact a behaviour change. + +Tests that reach into private methods and attributes are fragile in the other direction: they break on refactors that change nothing for customers, and they let a bug through as long as the internals look right. + +## Decision + +- An existing test is modified or deleted only when the PR demonstrates the old expectation was wrong. The description names the test, states what the old expectation asserted, and shows why that was a bug (a documented behaviour, an AWS contract, a reproduction). Otherwise the change is wrong, not the test. +- Every bug fix adds a regression test that fails on `develop` without the fix and passes with it. +- Functional tests never test internal methods. They exercise the public API the way a customer would: the decorators, classes and functions customers import, called with the events and arguments customers use. We want the test to prove real customer usage works, not that the internals look right. +- No calling private methods, patching private attributes or asserting on internal state. If behaviour cannot be observed through the public API, that is a design question for the PR, not a reason to poke inside. +- Sample events in `tests/events/` reflect what the AWS service really sends. When a fixture turns out to be wrong (for example a field the service never includes), we add a correct fixture rather than editing the old one to keep old tests green. +- Coverage of the lines a PR adds is checked before merge. New code without tests does not land. +- Known flaky tests are tracked and re-run; they are not deleted or loosened to pass. + +## Consequences + +- Reviews start by reading the test diff. A green suite with modified expectations gets more scrutiny, not less. +- Some fixes take longer because the bug has to be reproduced through the public surface first. That reproduction is the most valuable part of the PR. +- Refactors that change no behaviour should leave `tests/` untouched. + +## When reviewing or writing a change + +Run `git diff develop -- tests/` first. For every removed or changed assertion, find the sentence in the PR that justifies it. If there is none, ask. diff --git a/docs/adr/0006-zero-regression-and-compatibility.md b/docs/adr/0006-zero-regression-and-compatibility.md new file mode 100644 index 00000000000..df26eb40a6c --- /dev/null +++ b/docs/adr/0006-zero-regression-and-compatibility.md @@ -0,0 +1,29 @@ + +# 0006. Zero regression and backwards compatibility + +Status: Accepted + +## Context + +Customers upgrade Powertools for AWS Lambda (Python) through the layer or by bumping a pin, often without reading release notes. Code that worked on version N must work on N+1 within the same major. + +This includes behaviour we consider wrong but never rejected: an OpenAPI extension key without the `x-` prefix, a cookie attribute we emitted incorrectly, a lenient parse. Turning those into errors in a minor release breaks running functions. + +## Decision + +- A minor or patch release does not break code that worked on the previous one. Public signatures, return types, emitted payload shapes and exception types are kept. +- When input we accept today is invalid, a minor release warns with `PowertoolsUserWarning` and keeps the old behaviour. The warning states what will change and when. The error arrives in the next major. +- Removal of anything public happens only in a major release, after at least one minor with a deprecation warning and a migration path in `docs/upgrade.md`. +- Bug fixes may change behaviour when the old behaviour contradicted the documentation, the type hints or the AWS contract. The PR says so explicitly and the release notes call it out. +- New optional behaviour defaults to the old behaviour. New parameters have defaults that keep existing calls unchanged. +- Each change is checked against the previous release, not only against the test suite: what the old version returned for the same input, and whether a customer could depend on it. + +## Consequences + +- Some wrong behaviour lives for a while behind a warning. That is the cost of not breaking people. +- Release notes distinguish fixes that change observable behaviour from fixes that do not. +- Reviews ask "what did this return before, and who might rely on it?" before accepting a change in output. + +## When reviewing or writing a change + +Diff the behaviour, not only the code: run the same input on the previous release and on the branch. If the output differs, decide between warning and fix, and write the answer in the PR. diff --git a/docs/adr/0007-lean-dependencies.md b/docs/adr/0007-lean-dependencies.md new file mode 100644 index 00000000000..0dd4cb93d61 --- /dev/null +++ b/docs/adr/0007-lean-dependencies.md @@ -0,0 +1,29 @@ + +# 0007. Lean dependencies + +Status: Accepted + +## Context + +Every dependency we add is installed in every customer function that uses Powertools for AWS Lambda (Python), is part of their security surface, can conflict with versions they pin themselves, and adds to cold start ([ADR 0002](0002-every-millisecond-counts.md)). + +The core package depends on `jmespath` and `typing-extensions` only. Everything else is optional and installed through extras. + +## Decision + +- No new required dependency without a maintainer decision recorded in the issue. The default answer is no; the bar is "cannot be done reasonably without it". +- Optional functionality goes behind an extra in `pyproject.toml` and is imported lazily, so customers who do not use it do not install or load it. +- Version ranges are wide: a lower bound we tested and an upper bound at the next major. Narrow pins conflict with what customers already have in their functions. +- A dependency must support every Python version we support ([ADR 0003](0003-python-versions.md)) and both architectures. +- We do not vendor code to avoid a dependency unless the alternative is worse, and we do not add a dependency for something a few lines of standard library can do. +- Dependency bumps arrive through automation and are tested across the supported matrix before release. + +## Consequences + +- Features sometimes take more code than a library would need. That is accepted. +- The layer stays small and never carries what the Lambda runtime already provides ([ADR 0004](0004-layer-and-packaging.md)). +- Reviews of `pyproject.toml` and `uv.lock` changes are as careful as reviews of code. + +## When reviewing or writing a change + +If `pyproject.toml` changes, ask three questions: is it required or optional, is it imported lazily, and does it install cleanly on the oldest and newest supported Python on both architectures. diff --git a/docs/adr/README.md b/docs/adr/README.md new file mode 100644 index 00000000000..ea89b8f87ef --- /dev/null +++ b/docs/adr/README.md @@ -0,0 +1,20 @@ + +# Architecture decision records + +Premises of the project that are not visible in the code and that we found ourselves explaining more than once in reviews. They complement the [tenets](https://docs.powertools.aws.dev/lambda/python/latest/#tenets): tenets say what goes into the product, these say how we build it. + +Each record is one page: context, decision, consequences, and what it means when you review or write a change. Records are not edited once accepted; a new record supersedes an old one and the old one points forward. + +| ID | Title | Status | +| --- | --- | --- | +| [0001](0001-lambda-first.md) | Lambda first | Accepted | +| [0002](0002-every-millisecond-counts.md) | Every millisecond counts | Accepted | +| [0003](0003-python-versions.md) | Python versions follow the Lambda runtimes | Accepted | +| [0004](0004-layer-and-packaging.md) | Layer and packaging | Accepted | +| [0005](0005-tests-are-a-contract.md) | Tests are a contract | Accepted | +| [0006](0006-zero-regression-and-compatibility.md) | Zero regression and backwards compatibility | Accepted | +| [0007](0007-lean-dependencies.md) | Lean dependencies | Accepted | + +## Adding a record + +Copy the shape of an existing file, take the next number, and open a PR. A record is worth writing when a maintainer had to explain the same decision twice. Keep it to one page; link to the PRs or issues where the decision was made. diff --git a/mkdocs.yml b/mkdocs.yml index df695cb51c2..b4de38d80da 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -5,6 +5,10 @@ repo_url: https://github.com/aws-powertools/powertools-lambda-python edit_uri: edit/develop/docs site_url: https://docs.powertools.aws.dev/lambda/python/latest/ +# Architecture decision records live in the repository only, see docs/adr/README.md +exclude_docs: | + adr/ + nav: - Homepage: - index.md