MIL-004 request headers (Step 3) #35
@@ -21,7 +21,7 @@ The assignment is built in four lecture steps. Each step was delivered on its ow
|
|||||||
| --- | --- | --- |
|
| --- | --- | --- |
|
||||||
| 1 | Scrape the price from the practice page with BeautifulSoup | done |
|
| 1 | Scrape the price from the practice page with BeautifulSoup | done |
|
||||||
| 2 | E-mail alert below the target price, secrets in `.env` | done |
|
| 2 | E-mail alert below the target price, secrets in `.env` | done |
|
||||||
| 3 | Browser-like request headers | planned (MIL-004) |
|
| 3 | Browser-like request headers | done |
|
||||||
| 4 | The live Amazon page | planned (MIL-005) |
|
| 4 | The live Amazon page | planned (MIL-005) |
|
||||||
|
|
||||||
### Step 1: scrape the price
|
### Step 1: scrape the price
|
||||||
@@ -42,6 +42,16 @@ The screenshots below come from a run where the SMTP connection is replaced by a
|
|||||||
|
|
||||||

|

|
||||||
|
|
||||||
|
### Step 3: request headers
|
||||||
|
|
||||||
|
A browser sends headers with every request: the browser and system (`User-Agent`), the preferred language (`Accept-Language`) and more. A bare script sends almost none, so a server can answer in the wrong language or currency, or treat the request as a bot. `DEFAULT_HEADERS` in `constants.py` holds the browser-like set (without `Host` and `X-Amzn-Trace-Id`, as the lecture says) and `RequestsPageFetcher` passes it as `headers=` to `requests.get`.
|
||||||
|
|
||||||
|
What a browser sends to [httpbin.org/headers](https://httpbin.org/headers), and what the bot sends (`Host` is added by `requests` itself):
|
||||||
|
|
||||||
|

|
||||||
|
|
||||||
|

|
||||||
|
|
||||||
Repository layout:
|
Repository layout:
|
||||||
|
|
||||||
| Path | What it is for |
|
| Path | What it is for |
|
||||||
|
|||||||
@@ -15,7 +15,7 @@ document of a type. `Primary File` may contain a glob (e.g.
|
|||||||
| SA | Stakeholder Analysis | docs/stakeholder-analysis.md | 002 |
|
| SA | Stakeholder Analysis | docs/stakeholder-analysis.md | 002 |
|
||||||
| PP | Project Plan | docs/project-plan.md | 002 |
|
| PP | Project Plan | docs/project-plan.md | 002 |
|
||||||
| MIL | Milestone / Gateway | docs/milestones/*.md | 007 |
|
| MIL | Milestone / Gateway | docs/milestones/*.md | 007 |
|
||||||
| RC | SQA Review Record | docs/sqa/reviews/rc-*.md | 013 |
|
| RC | SQA Review Record | docs/sqa/reviews/rc-*.md | 014 |
|
||||||
|
|
||||||
## Languages
|
## Languages
|
||||||
|
|
||||||
|
|||||||
Binary file not shown.
|
After Width: | Height: | Size: 23 KiB |
Binary file not shown.
|
After Width: | Height: | Size: 25 KiB |
@@ -0,0 +1,62 @@
|
|||||||
|
# RC-013 Code review of MIL-004
|
||||||
|
|
||||||
|
## Metadata
|
||||||
|
| Key | Value |
|
||||||
|
| --- | --- |
|
||||||
|
| ID | RC-013 |
|
||||||
|
| CrossReference | [MIL-004], [QC-PY-001] |
|
||||||
|
|
||||||
|
## Version History
|
||||||
|
| Date | Status | Author | Reviewer | Change | Commit |
|
||||||
|
| --- | --- | --- | --- | --- | --- |
|
||||||
|
| 2026-10-28 | Proposed | Jens Tirsvad Nielsen | AI as stand-in for S01 | Initial version | [ec6ed21] |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Artifact Under Review
|
||||||
|
|
||||||
|
- Instance reviewed: [MIL-004]
|
||||||
|
- Checklist used: [QC-PY-001]
|
||||||
|
- Scope: full review
|
||||||
|
- Language and domain: n/a (technical)
|
||||||
|
- Language reviewer: none
|
||||||
|
- Mode: autonomous, not independent (AI as stand-in for S01); the author and the stand-in are both the AI working for S01
|
||||||
|
|
||||||
|
## Checklist Results
|
||||||
|
|
||||||
|
| # | Criterion | Status | Evidence/Notes |
|
||||||
|
| --- | --- | --- | --- |
|
||||||
|
| 1 | Packages, modules, functions, variables, classes and constants follow PEP 8 casing (`snake_case`, `PascalCase`, `UPPER_SNAKE`) | Pass | Modules and functions are snake_case, classes PascalCase, constants UPPER_SNAKE |
|
||||||
|
| 2 | Names state purpose in the domain's language; no unexplained abbreviations, no single-letter names outside tiny scopes | Pass | Names state purpose (price, target, headers); no abbreviations left unexplained |
|
||||||
|
| 3 | Code is produced by the project's formatter and passes its linter with no unexplained suppressions | Pass | ruff format and ruff check pass: 38 pytest tests, ruff 0 findings, mypy strict 0 errors (26 files) |
|
||||||
|
| 4 | Every function and method signature is type-annotated, including `-> None` | Pass | All signatures annotated; mypy --strict passes: 38 pytest tests, ruff 0 findings, mypy strict 0 errors (26 files) |
|
||||||
|
| 5 | No bare `except:`, no swallowed exceptions; specific exceptions are raised and the cause is kept (`raise ... from`) | Pass | No bare except; errors are specific and chained with `raise ... from` where a cause exists |
|
||||||
|
| 6 | No mutable default arguments and no shadowed builtins | Pass | No mutable defaults, no shadowed builtins |
|
||||||
|
| 7 | Files, locks and connections are managed with context managers | Pass | No open files or connections without a context manager |
|
||||||
|
| 8 | Public modules, classes and functions have docstrings that say what, not how | Pass | Public modules and functions have docstrings with Doxygen tags |
|
||||||
|
| 9 | Logging uses `logging`, not `print`; no secrets or personal data in log output | Pass | No secrets are involved; headers are public browser values |
|
||||||
|
| 10 | Classes and operations trace to the Design Class Diagram they implement; deviations are recorded | N-A | No Design Class Diagram exists; the milestone's technical tasks need none (Needs use case: No) |
|
||||||
|
| 11 | Tests exist for new behaviour, are named for the behaviour, and do not depend on order or the network | Pass | Tests exist, are named for behaviour and use no network or real mail server: 38 pytest tests, ruff 0 findings, mypy strict 0 errors (26 files) |
|
||||||
|
| 12 | Type checker runs in strict mode without errors; `Any` is justified in a comment | Pass | mypy --strict passes; no Any |
|
||||||
|
| 13 | Dependencies are declared and pinned in the project's dependency file, none unused | Pass | Dependencies are in pyproject.toml with lower bounds; none unused |
|
||||||
|
| 14 | Each piece of knowledge (a business rule, constant, format, validation or query) is defined in one place; code is merged only where it expresses the same knowledge, not where it merely looks alike | Pass | Constants live only in constants.py |
|
||||||
|
| 15 | Imports point inward: business-rule modules import no framework, database, UI or delivery-mechanism code, and there are no import cycles | Pass | Imports point inward; no cycles |
|
||||||
|
| 16 | SOLID holds: a class or module has one reason to change; new behaviour is added by extension, not by editing a type test in several places; subclasses honour the contract of their base; protocols are narrow; high-level code depends on protocols or abstract types and receives its implementations | Pass | The fetcher takes its headers as a constructor argument (extension point); the default is one read-only mapping in constants.py |
|
||||||
|
| 17 | Source is laid out in the layers and folders of the clean architecture folder structure (`domain`, `application`, `infrastructure`, `presentation` packages under `src/`, and `unit`, `integration` and `e2e` under `tests/`), in `snake_case`, and imports between the layers point inward | Pass | Layers and tests/unit/ mirror as before; only constants and the infrastructure adapter changed |
|
||||||
|
| 18 | File and folder names are English, contain no space and no character outside letters, digits, `-`, `_` and `.`, and follow the casing of the language for files | Pass | File names are English snake_case |
|
||||||
|
|
||||||
|
## Overall Verdict
|
||||||
|
|
||||||
|
Go — all Mandatory criteria pass or are not applicable.
|
||||||
|
|
||||||
|
## Action Items
|
||||||
|
|
||||||
|
| Action | Owner | Due |
|
||||||
|
| --- | --- | --- |
|
||||||
|
| None: no Fail and no Pending criterion | S01 | 2026-10-28 |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
[MIL-004]: ../../milestones/mil-004-request-headers.md
|
||||||
|
[QC-PY-001]: ../../../framework/qc/code/qc-programming-python.md
|
||||||
|
[ec6ed21]: https://git.tirsystem.com/Tirsvad-Udemy-100-days-of-code/047-amazon-price-tracker/commit/ec6ed213fbbc33941caf31019cee42997deb4261
|
||||||
@@ -4,6 +4,8 @@
|
|||||||
@brief The only place that defines constants (URLs, limits, environment names).
|
@brief The only place that defines constants (URLs, limits, environment names).
|
||||||
"""
|
"""
|
||||||
|
|
||||||
|
from collections.abc import Mapping
|
||||||
|
from types import MappingProxyType
|
||||||
from typing import Final
|
from typing import Final
|
||||||
|
|
||||||
## @brief Practice page used instead of Amazon while developing.
|
## @brief Practice page used instead of Amazon while developing.
|
||||||
@@ -23,6 +25,30 @@ TARGET_PRICE: Final[float] = 100.0
|
|||||||
## @brief Seconds to wait for an HTTP response.
|
## @brief Seconds to wait for an HTTP response.
|
||||||
REQUEST_TIMEOUT_SECONDS: Final[int] = 10
|
REQUEST_TIMEOUT_SECONDS: Final[int] = 10
|
||||||
|
|
||||||
|
## @brief Browser-like request headers, read-only.
|
||||||
|
# Copied from a real browser (see httpbin.org/headers) as the lecture says,
|
||||||
|
# without Host and X-Amzn-Trace-Id. No br/zstd: requests may not decode them.
|
||||||
|
DEFAULT_HEADERS: Final[Mapping[str, str]] = MappingProxyType(
|
||||||
|
{
|
||||||
|
"User-Agent": (
|
||||||
|
"Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 "
|
||||||
|
"(KHTML, like Gecko) Chrome/126.0.0.0 Safari/537.36"
|
||||||
|
),
|
||||||
|
"Accept-Language": "en-GB,en-US;q=0.9,en;q=0.8",
|
||||||
|
"Accept": (
|
||||||
|
"text/html,application/xhtml+xml,application/xml;q=0.9,"
|
||||||
|
"image/avif,image/webp,*/*;q=0.8"
|
||||||
|
),
|
||||||
|
"Accept-Encoding": "gzip, deflate",
|
||||||
|
"Upgrade-Insecure-Requests": "1",
|
||||||
|
"Sec-Fetch-Dest": "document",
|
||||||
|
"Sec-Fetch-Mode": "navigate",
|
||||||
|
"Sec-Fetch-Site": "none",
|
||||||
|
"Sec-Fetch-User": "?1",
|
||||||
|
"Priority": "u=0, i",
|
||||||
|
}
|
||||||
|
)
|
||||||
|
|
||||||
## @brief SMTP submission port (STARTTLS).
|
## @brief SMTP submission port (STARTTLS).
|
||||||
SMTP_PORT: Final[int] = 587
|
SMTP_PORT: Final[int] = 587
|
||||||
|
|
||||||
|
|||||||
@@ -4,14 +4,23 @@
|
|||||||
@brief Downloads a page with requests.
|
@brief Downloads a page with requests.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
|
from collections.abc import Mapping
|
||||||
|
|
||||||
import requests
|
import requests
|
||||||
|
|
||||||
from amazon_price_tracker.constants import REQUEST_TIMEOUT_SECONDS
|
from amazon_price_tracker.constants import DEFAULT_HEADERS, REQUEST_TIMEOUT_SECONDS
|
||||||
|
|
||||||
|
|
||||||
class RequestsPageFetcher:
|
class RequestsPageFetcher:
|
||||||
"""PageFetcher port implemented with requests."""
|
"""PageFetcher port implemented with requests."""
|
||||||
|
|
||||||
|
def __init__(self, headers: Mapping[str, str] = DEFAULT_HEADERS) -> None:
|
||||||
|
"""Keep the headers sent with every request.
|
||||||
|
|
||||||
|
@param headers Request headers; browser-like by default.
|
||||||
|
"""
|
||||||
|
self._headers = headers
|
||||||
|
|
||||||
def fetch(self, url: str) -> str:
|
def fetch(self, url: str) -> str:
|
||||||
"""Return the HTML text of the page at url.
|
"""Return the HTML text of the page at url.
|
||||||
|
|
||||||
@@ -19,6 +28,8 @@ class RequestsPageFetcher:
|
|||||||
@return The response body as text.
|
@return The response body as text.
|
||||||
@throws requests.HTTPError When the server answers with an error status.
|
@throws requests.HTTPError When the server answers with an error status.
|
||||||
"""
|
"""
|
||||||
response = requests.get(url, timeout=REQUEST_TIMEOUT_SECONDS)
|
response = requests.get(
|
||||||
|
url, headers=dict(self._headers), timeout=REQUEST_TIMEOUT_SECONDS
|
||||||
|
)
|
||||||
response.raise_for_status()
|
response.raise_for_status()
|
||||||
return response.text
|
return response.text
|
||||||
|
|||||||
@@ -1,9 +1,11 @@
|
|||||||
import pytest
|
import pytest
|
||||||
import requests
|
import requests
|
||||||
|
|
||||||
from amazon_price_tracker.constants import REQUEST_TIMEOUT_SECONDS
|
from amazon_price_tracker.constants import DEFAULT_HEADERS, REQUEST_TIMEOUT_SECONDS
|
||||||
from amazon_price_tracker.infrastructure.http_client import RequestsPageFetcher
|
from amazon_price_tracker.infrastructure.http_client import RequestsPageFetcher
|
||||||
|
|
||||||
|
Call = tuple[str, dict[str, str], float]
|
||||||
|
|
||||||
|
|
||||||
class FakeResponse:
|
class FakeResponse:
|
||||||
def __init__(self, text: str, status_code: int) -> None:
|
def __init__(self, text: str, status_code: int) -> None:
|
||||||
@@ -15,13 +17,11 @@ class FakeResponse:
|
|||||||
raise requests.HTTPError(f"status {self.status_code}")
|
raise requests.HTTPError(f"status {self.status_code}")
|
||||||
|
|
||||||
|
|
||||||
def _patch_get(
|
def _patch_get(monkeypatch: pytest.MonkeyPatch, response: FakeResponse) -> list[Call]:
|
||||||
monkeypatch: pytest.MonkeyPatch, response: FakeResponse
|
calls: list[Call] = []
|
||||||
) -> list[tuple[str, float]]:
|
|
||||||
calls: list[tuple[str, float]] = []
|
|
||||||
|
|
||||||
def fake_get(url: str, timeout: float) -> FakeResponse:
|
def fake_get(url: str, headers: dict[str, str], timeout: float) -> FakeResponse:
|
||||||
calls.append((url, timeout))
|
calls.append((url, headers, timeout))
|
||||||
return response
|
return response
|
||||||
|
|
||||||
monkeypatch.setattr(requests, "get", fake_get)
|
monkeypatch.setattr(requests, "get", fake_get)
|
||||||
@@ -36,7 +36,46 @@ def test_fetch_returns_text_using_url_and_timeout(
|
|||||||
html = RequestsPageFetcher().fetch("https://example.test/")
|
html = RequestsPageFetcher().fetch("https://example.test/")
|
||||||
|
|
||||||
assert html == "<p>hi</p>"
|
assert html == "<p>hi</p>"
|
||||||
assert calls == [("https://example.test/", REQUEST_TIMEOUT_SECONDS)]
|
assert calls == [
|
||||||
|
("https://example.test/", dict(DEFAULT_HEADERS), REQUEST_TIMEOUT_SECONDS)
|
||||||
|
]
|
||||||
|
|
||||||
|
|
||||||
|
def test_fetch_sends_user_agent_and_accept_language(
|
||||||
|
monkeypatch: pytest.MonkeyPatch,
|
||||||
|
) -> None:
|
||||||
|
calls = _patch_get(monkeypatch, FakeResponse("", 200))
|
||||||
|
|
||||||
|
RequestsPageFetcher().fetch("https://example.test/")
|
||||||
|
|
||||||
|
headers = calls[0][1]
|
||||||
|
assert headers["User-Agent"]
|
||||||
|
assert headers["Accept-Language"]
|
||||||
|
|
||||||
|
|
||||||
|
def test_fetch_omits_host_and_trace_id_headers(
|
||||||
|
monkeypatch: pytest.MonkeyPatch,
|
||||||
|
) -> None:
|
||||||
|
calls = _patch_get(monkeypatch, FakeResponse("", 200))
|
||||||
|
|
||||||
|
RequestsPageFetcher().fetch("https://example.test/")
|
||||||
|
|
||||||
|
assert "Host" not in calls[0][1]
|
||||||
|
assert "X-Amzn-Trace-Id" not in calls[0][1]
|
||||||
|
|
||||||
|
|
||||||
|
def test_fetch_uses_overriding_headers(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||||
|
calls = _patch_get(monkeypatch, FakeResponse("", 200))
|
||||||
|
|
||||||
|
RequestsPageFetcher(headers={"User-Agent": "custom"}).fetch("https://example.test/")
|
||||||
|
|
||||||
|
assert calls[0][1] == {"User-Agent": "custom"}
|
||||||
|
assert calls[0][2] == REQUEST_TIMEOUT_SECONDS
|
||||||
|
|
||||||
|
|
||||||
|
def test_default_headers_are_read_only() -> None:
|
||||||
|
with pytest.raises(TypeError):
|
||||||
|
DEFAULT_HEADERS["User-Agent"] = "x" # type: ignore[index]
|
||||||
|
|
||||||
|
|
||||||
def test_fetch_raises_on_http_error(monkeypatch: pytest.MonkeyPatch) -> None:
|
def test_fetch_raises_on_http_error(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||||
|
|||||||
@@ -36,6 +36,12 @@ def test_env_names_are_unique() -> None:
|
|||||||
assert len(set(ENV_NAMES)) == len(ENV_NAMES)
|
assert len(set(ENV_NAMES)) == len(ENV_NAMES)
|
||||||
|
|
||||||
|
|
||||||
|
def test_default_headers_are_non_empty() -> None:
|
||||||
|
"""Every default header has a name and a value."""
|
||||||
|
assert constants.DEFAULT_HEADERS
|
||||||
|
assert all(k.strip() and v.strip() for k, v in constants.DEFAULT_HEADERS.items())
|
||||||
|
|
||||||
|
|
||||||
def test_smtp_port_is_starttls_port() -> None:
|
def test_smtp_port_is_starttls_port() -> None:
|
||||||
"""The SMTP port is 587."""
|
"""The SMTP port is 587."""
|
||||||
assert constants.SMTP_PORT == 587
|
assert constants.SMTP_PORT == 587
|
||||||
|
|||||||
Reference in New Issue
Block a user