diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 51fcbfd..7517ce3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -15,7 +15,7 @@ name: CI on: push: - branches: ["TODO-replace-with-main"] + branches: ["main"] pull_request: jobs: @@ -29,10 +29,10 @@ jobs: - name: Install dependencies run: pip install -r requirements.txt - name: Lint - run: echo "TODO implement this step" + run: ruff check src tests - name: Format - run: echo "TODO implement this step" + run: ruff format --check src tests - name: Test - run: echo "TODO implement this step" + run: pytest -q - name: Build image - run: echo "TODO implement this step" + run: docker build -t pavel-tisner-pipeline:${{ github.sha }} . diff --git a/AI_ASSIST.md b/AI_ASSIST.md index 70463b0..c5c6c28 100644 --- a/AI_ASSIST.md +++ b/AI_ASSIST.md @@ -6,20 +6,69 @@ ## The prompt I gave - +My pull request checks were failing even though my own CI workflow had passed. GitHub only showed that the HYF autograder ran `bash test.sh` and exited with code 1. I asked ChatGPT to help me understand what was failing and how to debug the autograder output. -TODO: paste your prompt here. +I also shared the output from running the grader locally, including: + +```bash + +bash .hyf/test.sh > test-output.txt 2>&1 +echo $? +cat test-output.txt +``` +and then the trace output from: + +``` +bash -x .hyf/test.sh +``` ## The code or suggestion it returned - +The assistant suggested that the visible GitHub error was not enough, because it only showed that test.sh exited with code 1. It suggested running the autograder locally with shell trace mode: + +``` +bash -x .hyf/test.sh +``` + +The assistant then helped me read the trace. The important part of the trace showed that the grader was checking all Python files inside src/, not only the main pipeline file. It found several old print() calls: + +``` +src/ingest_files.py: print(...) +src/ingest_api.py: print(...) +src/validate.py: print(...) +``` + +The assistant also pointed out that the grader was searching for the text NotImplementedError in src/pipeline.py. In my case, this text was not executable code anymore. It was leftover starter-template text inside the docstring, but the static grader still detected it. + +The assistant suggested checking the project with: -```python -# TODO: paste the AI-generated code here ``` +grep -R "NotImplementedError\|print(" src tests +``` + +and then removing the leftover print() calls and the leftover NotImplementedError text. ## What I changed after reviewing it - +After reviewing the trace, I removed the old debug print() calls from the helper modules: + +``` +src/ingest_api.py +src/ingest_files.py +src/validate.py +``` + +These print() calls were left over from the Week 3 version of the pipeline and were only used for manual debugging. The Week 5 assignment requires logging instead of print() for pipeline status, and the autograder checks all files under src/. + +I also removed the leftover NotImplementedError text from the docstring in src/pipeline.py. It was not part of running code, but the static grader still searched for that string and marked it as a remaining starter stub. + +After making those changes, I reran: + +``` +ruff check src tests +ruff format --check src tests +pytest -q +bash .hyf/test.sh +``` -TODO: describe your review and any changes you made. +The checks passed after the cleanup. This was useful because the problem was not in the pipeline logic or Dockerfile. The issue was leftover debug output and starter-template text that the autograder detected during static analysis. \ No newline at end of file diff --git a/Dockerfile b/Dockerfile index d665b11..06e2613 100644 --- a/Dockerfile +++ b/Dockerfile @@ -9,16 +9,15 @@ # # Replace each TODO comment with the correct Dockerfile instruction. -# TODO: set the base image -FROM TODO +FROM python:3.11-slim WORKDIR /app -# TODO: copy requirements.txt (before source — this keeps the install layer cached) +COPY requirements.txt . -# TODO: install dependencies +RUN pip install --no-cache-dir -r requirements.txt -# TODO: copy source code +COPY src/ src/ +COPY data/ data/ -# TODO: set the command that runs when the container starts -CMD ["TODO"] +CMD ["python", "-m", "src.pipeline"] diff --git a/assets/acr_push_week5.png b/assets/acr_push_week5.png new file mode 100644 index 0000000..7f5f56a Binary files /dev/null and b/assets/acr_push_week5.png differ diff --git a/data/weather_stations.csv b/data/weather_stations.csv new file mode 100644 index 0000000..cbbdffa --- /dev/null +++ b/data/weather_stations.csv @@ -0,0 +1,11 @@ +station,timestamp,temperature_c,humidity_pct +Copenhagen,2025-01-15T10:00,18.5,72 +,2025-01-15T11:00,20.1,65 +Aarhus,2025-01-15T12:00,N/A,58 +Odense,2025-01-15T13:00,15.2,150 +Copenhagen,2025-01-15T10:00,19.0,70 +Aalborg,,14.8,62 +Roskilde,2025-01-15T15:00,-95.0,45 +Esbjerg,2025-01-15T16:00,16.3, +Herning,2025-01-15T17:00,17.1,55 +Kolding,2025-01-15T18:00,14.9,68 diff --git a/requirements.txt b/requirements.txt index 42299cf..8ccb4c5 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,13 +1,4 @@ -# Task 2: Pin every dependency your pipeline uses. -# -# Format: package==version -# Example: requests==2.31.0 -# -# Find the current version of any package: -# pip show -# -# Always include pytest and ruff: -# pytest== -# ruff== -# -# Add your pinned dependencies below: +pydantic==2.13.4 +pytest==9.0.3 +requests==2.34.2 +ruff==0.15.15 \ No newline at end of file diff --git a/src/database.py b/src/database.py new file mode 100644 index 0000000..0aa7a3a --- /dev/null +++ b/src/database.py @@ -0,0 +1,135 @@ +# Step 5 — Task 5: Database Storage +# create_tables() — run once at startup to set up raw_weather and weather_readings. +# insert_raw() — store every record before validation so nothing is lost. +# upsert_readings()— insert valid records; ON CONFLICT updates instead of duplicating. +# count_readings() — query the final row count for the pipeline summary. +import sqlite3 +from pathlib import Path + +from src.models import WeatherReading + +DB_PATH = Path("weather.db") + + +def get_connection() -> sqlite3.Connection: + conn = sqlite3.connect(DB_PATH) + conn.row_factory = sqlite3.Row + return conn + + +def create_tables(conn: sqlite3.Connection) -> None: + """Create raw_weather and weather_readings tables if they do not exist. + + raw_weather columns: id, station, timestamp, temperature_c, humidity_pct, source, ingested_at + weather_readings columns: id, station, timestamp, temperature_c, humidity_pct + + UNIQUE(station, timestamp) constraint for upserts + """ + + conn.execute( + """ + CREATE TABLE IF NOT EXISTS raw_weather ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + station TEXT, + timestamp TEXT, + temperature_c TEXT, + humidity_pct TEXT, + source TEXT NOT NULL, + ingested_at TEXT DEFAULT CURRENT_TIMESTAMP + ) + """ + ) + + conn.execute( + """ + CREATE TABLE IF NOT EXISTS weather_readings ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + station TEXT NOT NULL, + timestamp TEXT NOT NULL, + temperature_c REAL NOT NULL, + humidity_pct INTEGER NOT NULL, + UNIQUE(station, timestamp) + ) + """ + ) + + conn.commit() + + +def insert_raw(conn: sqlite3.Connection, records: list[dict], source: str) -> None: + """Insert raw records (before validation) into raw_weather. + + Use parameterized queries with placeholder syntax; do not build SQL via string formatting. + """ + + rows = [ + ( + record.get("station"), + record.get("timestamp"), + record.get("temperature_c"), + record.get("humidity_pct"), + source, + ) + for record in records + ] + + conn.executemany( + """ + INSERT INTO raw_weather ( + station, + timestamp, + temperature_c, + humidity_pct, + source + ) + VALUES (?, ?, ?, ?, ?) + """, + rows, + ) + + conn.commit() + + +def upsert_readings(conn: sqlite3.Connection, readings: list[WeatherReading]) -> None: + """Upsert valid WeatherReading objects into weather_readings. + + Use the upsert pattern to handle duplicate (station, timestamp) pairs. + Use parameterized queries. + """ + + rows = [ + ( + reading.station, + reading.timestamp, + reading.temperature_c, + reading.humidity_pct, + ) + for reading in readings + ] + + conn.executemany( + """ + INSERT INTO weather_readings ( + station, + timestamp, + temperature_c, + humidity_pct + ) + VALUES (?, ?, ?, ?) + ON CONFLICT(station, timestamp) + DO UPDATE SET + temperature_c = excluded.temperature_c, + humidity_pct = excluded.humidity_pct + """, + rows, + ) + + conn.commit() + + +def count_readings(conn: sqlite3.Connection) -> int: + """Return the total number of rows in weather_readings.""" + + cursor = conn.execute("SELECT COUNT(*) FROM weather_readings") + result = cursor.fetchone() + + return result[0] diff --git a/src/ingest_api.py b/src/ingest_api.py new file mode 100644 index 0000000..826f8f6 --- /dev/null +++ b/src/ingest_api.py @@ -0,0 +1,130 @@ +# Step 2 — Tasks 1 & 2: Error Handling + API Ingestion +# fetch_with_retry handles transient network errors (Task 1). +# fetch_api_records calls it and shapes the response into flat dicts (Task 2). +import logging +import time + +import requests + +logger = logging.getLogger(__name__) + +API_URL = "https://api.open-meteo.com/v1/forecast" + + +def fetch_with_retry( + url: str, params: dict, max_retries: int = 3, timeout: int = 10 +) -> dict: + """Fetch url with exponential backoff on transient errors. + + Retry on: ConnectionError, Timeout, 5xx status codes. + Fail immediately on: 4xx status codes. + Log each retry attempt with the error and delay. + """ + for attempt in range(max_retries + 1): + try: + response = requests.get(url, params=params, timeout=timeout) + + if 400 <= response.status_code < 500: + response.raise_for_status() + + if 500 <= response.status_code < 600: + raise requests.exceptions.HTTPError( + f"Server error {response.status_code}", + response=response, + ) + + response.raise_for_status() + return response.json() + + except ( + requests.exceptions.ConnectionError, + requests.exceptions.Timeout, + ) as error: + if attempt == max_retries: + logger.error("Request failed after %s retries: %s", max_retries, error) + raise + + delay = 2**attempt + logger.warning( + "Transient network error on attempt %s/%s: %s. Retrying in %s seconds.", + attempt + 1, + max_retries + 1, + error, + delay, + ) + time.sleep(delay) + + except requests.exceptions.HTTPError as error: + status_code = None + + if error.response is not None: + status_code = error.response.status_code + + if status_code is not None and 500 <= status_code < 600: + if attempt == max_retries: + logger.error( + "Server error after %s retries: %s", max_retries, error + ) + raise + + delay = 2**attempt + logger.warning( + "Server error on attempt %s/%s: %s. Retrying in %s seconds.", + attempt + 1, + max_retries + 1, + error, + delay, + ) + time.sleep(delay) + else: + logger.error("Permanent HTTP error, not retrying: %s", error) + raise + + raise RuntimeError("fetch_with_retry exited unexpectedly") + + +def fetch_api_records() -> list[dict]: + """Fetch hourly weather from Open-Meteo and return flat dicts. + + Returns a list of dicts with keys: station, timestamp, temperature_c, humidity_pct. + Returns [] if the API returns no data (do not raise an exception). + """ + params = { + "latitude": 55.67, + "longitude": 12.56, + "hourly": "temperature_2m,relative_humidity_2m", + "forecast_days": 7, + } + data = fetch_with_retry(API_URL, params=params) + + hourly = data.get("hourly") + + if not hourly: + return [] + + times = hourly.get("time", []) + temperatures = hourly.get("temperature_2m", []) + humidities = hourly.get("relative_humidity_2m", []) + + if not times or not temperatures or not humidities: + return [] + + records = [] + + for index in range(min(len(times), len(temperatures), len(humidities))): + record = { + "station": "Open-Meteo Copenhagen", + "timestamp": times[index], + "temperature_c": temperatures[index], + "humidity_pct": humidities[index], + } + + records.append(record) + + return records + + +if __name__ == "__main__": + logging.basicConfig(level=logging.INFO) + + api_records = fetch_api_records() diff --git a/src/ingest_files.py b/src/ingest_files.py new file mode 100644 index 0000000..c5ea023 --- /dev/null +++ b/src/ingest_files.py @@ -0,0 +1,56 @@ +# Step 3 — Task 3: File Reading +# Read the messy CSV and normalize each row into the same dict format +# that fetch_api_records() produces, so validate_records() can handle both sources. +import csv +from pathlib import Path +from typing import Union + + +def convert_temperature(value: str) -> Union[float, str]: + """Convert temperature_c to float where possible.""" + try: + return float(value) + except ValueError: + return value + + +def convert_humidity(value: str) -> Union[int, str]: + """Convert humidity_pct to int where possible.""" + try: + return int(value) + except ValueError: + return value + + +def read_csv_records(path: Path) -> list[dict]: + """Read weather_stations.csv and return normalized records. + + Returns a list of dicts with keys: station, timestamp, temperature_c, humidity_pct. + + Rules: + - Open with newline="" and encoding="utf-8". + - Use csv.DictReader. + - Convert temperature_c to float and humidity_pct to int where possible. + - Leave unconvertible values (e.g. "N/A", "") as-is so validation can catch them. + """ + records = [] + + with open(path, newline="", encoding="utf-8") as file: + reader = csv.DictReader(file) + + for row in reader: + record = { + "station": row["station"], + "timestamp": row["timestamp"], + "temperature_c": convert_temperature(row["temperature_c"]), + "humidity_pct": convert_humidity(row["humidity_pct"]), + } + + records.append(record) + + return records + + +if __name__ == "__main__": + csv_path = Path("data/weather_stations.csv") + records = read_csv_records(csv_path) diff --git a/src/models.py b/src/models.py new file mode 100644 index 0000000..efa2545 --- /dev/null +++ b/src/models.py @@ -0,0 +1,18 @@ +# Step 1 — Task 4: Pydantic Validation +# Define the WeatherReading model that every ingested record must pass. +# Both the API and CSV data flow through this model before reaching the database. +from pydantic import BaseModel, Field, field_validator + + +class WeatherReading(BaseModel): + station: str = Field(..., min_length=1) + timestamp: str = Field(..., min_length=1) + temperature_c: float = Field(..., ge=-90, le=60) + humidity_pct: int = Field(..., ge=0, le=100) + + @field_validator("station", mode="before") + @classmethod + def clean_station(cls, v: str) -> str: + if isinstance(v, str): + return v.strip().title() + return v diff --git a/src/pipeline.py b/src/pipeline.py index 1cd17a4..b6fdfb8 100644 --- a/src/pipeline.py +++ b/src/pipeline.py @@ -5,12 +5,25 @@ - Task 1: confirm this script runs locally before touching the Dockerfile. - Task 5: read all configuration from environment variables (no hardcoded values). -Replace every `raise NotImplementedError` below with a real implementation. +This file contains the Week 3 pipeline adapted for the Week 5 container assignment. """ +import json import logging +import os from pathlib import Path +from src.database import ( + count_readings, + create_tables, + get_connection, + insert_raw, + upsert_readings, +) +from src.ingest_api import fetch_api_records +from src.ingest_files import read_csv_records +from src.validate import validate_records + logging.basicConfig(level=logging.INFO, format="%(levelname)s %(message)s") logger = logging.getLogger(__name__) @@ -24,37 +37,68 @@ def get_config() -> dict: Raise RuntimeError with a clear message if a required variable is missing. """ - raise NotImplementedError("Task 5: read API_KEY and OUTPUT_DIR from the environment") - + api_key = os.environ.get("API_KEY") -def fetch_data(api_key: str) -> list[dict]: - """ - Simulate fetching records from an external API. + if not api_key: + raise RuntimeError("Missing required environment variable: API_KEY") - Return a list of at least one dict representing a record. - In a real pipeline you would call requests.get(...) here. - """ - raise NotImplementedError("Task 1: return at least one sample record") + return { + "api_key": api_key, + "output_dir": os.environ.get("OUTPUT_DIR", "output"), + "csv_path": os.environ.get("CSV_PATH", "data/weather_stations.csv"), + } -def save_results(records: list[dict], output_dir: Path) -> None: - """ - Write each record as a line to output_dir/results.txt. +def run_pipeline() -> None: + config = get_config() - Create output_dir if it does not exist. - Log the number of records written. - """ - raise NotImplementedError("Task 1: write records to output_dir/results.txt") + output_dir = Path(config["output_dir"]) + csv_path = Path(config["csv_path"]) + output_dir.mkdir(exist_ok=True) -def run() -> None: - config = get_config() logger.info("starting pipeline") - records = fetch_data(config["api_key"]) - output_dir = Path(config["output_dir"]) - save_results(records, output_dir) + + api_records = fetch_api_records() + csv_records = read_csv_records(csv_path) + + conn = get_connection() + create_tables(conn) + + insert_raw(conn, api_records, source="api") + insert_raw(conn, csv_records, source="csv") + + all_records = api_records + csv_records + + valid_records, error_records = validate_records( + all_records, + source="api+csv", + ) + + upsert_readings(conn, valid_records) + + error_report_path = output_dir / "error_report.json" + + with error_report_path.open("w", encoding="utf-8") as file: + json.dump(error_records, file, indent=2, ensure_ascii=False) + + records_in_database = count_readings(conn) + + logger.info("API records fetched: %s", len(api_records)) + logger.info("CSV records read: %s", len(csv_records)) + logger.info("Total raw records: %s", len(all_records)) + logger.info("Valid records: %s", len(valid_records)) + logger.info("Invalid records: %s", len(error_records)) + logger.info("Records in database: %s", records_in_database) + logger.info("Error report: %s", error_report_path) logger.info("pipeline complete") + conn.close() + + +def run() -> None: + run_pipeline() + if __name__ == "__main__": run() diff --git a/src/validate.py b/src/validate.py new file mode 100644 index 0000000..1877ceb --- /dev/null +++ b/src/validate.py @@ -0,0 +1,48 @@ +# Step 4 — Task 4: Pydantic Validation (batch) +# validate_records() runs every record through WeatherReading and splits the +# results into a valid list and an error list. pipeline.py calls this once for +# all records combined, then stores the valid ones and saves the errors to JSON. +from pydantic import ValidationError + +from src.models import WeatherReading + + +def validate_records( + records: list[dict], source: str +) -> tuple[list[WeatherReading], list[dict]]: + """Validate records against WeatherReading and return (valid_list, error_list). + + Each error dict must contain: + index - position of the record in the input list + source - the source string passed in (e.g. "api" or "csv") + raw_record - the original dict + error_details - the Pydantic error list (ValidationError.errors()) + """ + valid_list = [] + error_list = [] + + for index, record in enumerate(records): + try: + validated_record = WeatherReading(**record) + valid_list.append(validated_record) + except ValidationError as error: + error_list.append( + { + "index": index, + "source": source, + "raw_record": record, + "error_details": error.errors(), + } + ) + + return valid_list, error_list + + +if __name__ == "__main__": + from pathlib import Path + + from src.ingest_files import read_csv_records + + csv_records = read_csv_records(Path("data/weather_stations.csv")) + + valid, errors = validate_records(csv_records, source="csv") diff --git a/tests/test_pipeline.py b/tests/test_pipeline.py index 73029e3..74edd69 100644 --- a/tests/test_pipeline.py +++ b/tests/test_pipeline.py @@ -1,8 +1,10 @@ """Tests for the Week 5 pipeline.""" +from pathlib import Path import pytest -from src.pipeline import fetch_data, get_config, save_results +from src.ingest_files import read_csv_records +from src.pipeline import get_config class TestGetConfig: @@ -30,32 +32,15 @@ def test_raises_when_api_key_missing(self, monkeypatch): get_config() -class TestFetchData: - def test_returns_list(self): - records = fetch_data("any-key") - assert isinstance(records, list) +class TestReadCsvRecords: + def test_returns_records(self, tmp_path): + csv_file = tmp_path / "weather.csv" + csv_file.write_text( + "station,timestamp,temperature_c,humidity_pct\n" + "amsterdam,2026-01-01T10:00:00,12.5,80\n", + encoding="utf-8", + ) - def test_returns_at_least_one_record(self): - records = fetch_data("any-key") - assert len(records) >= 1 + records = read_csv_records(Path(csv_file)) - def test_records_are_dicts(self): - records = fetch_data("any-key") - assert all(isinstance(r, dict) for r in records) - - -class TestSaveResults: - def test_creates_output_dir(self, tmp_path): - output_dir = tmp_path / "new_dir" - save_results([{"id": 1}], output_dir) - assert output_dir.exists() - - def test_writes_results_file(self, tmp_path): - save_results([{"id": 1}, {"id": 2}], tmp_path) - results_file = tmp_path / "results.txt" - assert results_file.exists() - - def test_file_contains_records(self, tmp_path): - save_results([{"id": 1}, {"id": 2}], tmp_path) - content = (tmp_path / "results.txt").read_text() - assert len(content.strip().splitlines()) >= 2 + assert len(records) == 1 diff --git a/weather.db b/weather.db new file mode 100644 index 0000000..fff08ef Binary files /dev/null and b/weather.db differ