diff --git a/.env.example b/.env.example new file mode 100644 index 0000000..8e4f793 --- /dev/null +++ b/.env.example @@ -0,0 +1,5 @@ +# Copy to .env (git-ignored). Never commit real values. +DATABRICKS_HOST=adb-xxxx.azuredatabricks.net +DATABRICKS_HTTP_PATH=/sql/1.0/warehouses/xxxxxxxx +DATABRICKS_TOKEN=dapi...your-token-here +DBT_SCHEMA=dev_yourname diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md new file mode 100644 index 0000000..f2bd07e --- /dev/null +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -0,0 +1,15 @@ +## Summary +- **Databricks Job Run URL:** `` +- **Task 1 (PySpark):** Notebook in `task-1/` with aggregated borough and payment type queries. +- **Task 2 (dbt Incremental):** Ported dbt project in `task-2/` with `materialized='incremental'`, `merge` strategy, timing comparison, and `DESCRIBE HISTORY` proof in `WRITEUP.md`. +- **Task 3 (Job Scheduling):** Scheduled Databricks Job from GitHub fork on `hyf-dbt-warehouse` with screenshots and orchestration comparison in `task-3/SCHEDULING.md`. + +## How to review +- Open `task-2/WRITEUP.md` to check initial vs incremental build times and `DESCRIBE HISTORY` output. +- Open `task-3/SCHEDULING.md` to verify the Databricks Job Run URL, screenshots, and Airflow comparison. +- Open `AI_ASSIST.md` for documented LLM interactions. + +## Secrets hygiene checklist +- [ ] No `.env` or `profiles.yml` files committed. +- [ ] No Databricks personal access tokens (`dapi...`) hardcoded in any notebook or SQL file. +- [ ] `profiles.yml.example` and `.env.example` templates present. diff --git a/.github/workflows/grade-assignment.yml b/.github/workflows/grade-assignment.yml index d06dc2d..ed3644e 100644 --- a/.github/workflows/grade-assignment.yml +++ b/.github/workflows/grade-assignment.yml @@ -8,6 +8,11 @@ on: jobs: grade: permissions: + contents: read issues: write pull-requests: write - uses: HackYourFuture/github-actions/.github/workflows/auto-grade.yml@main + # Temporary pin: actions/checkout v6 blocks fork checkouts under + # pull_request_target unless allow-unsafe-pr-checkout is set. + # Revert to HackYourFuture/github-actions@main after + # https://github.com/HackYourFuture/github-actions/pull/4 merges. + uses: lassebenni/github-actions-fork/.github/workflows/auto-grade.yml@fix/allow-unsafe-pr-checkout-for-autograde diff --git a/.github/workflows/pr-body-check.yml b/.github/workflows/pr-body-check.yml new file mode 100644 index 0000000..c3870f1 --- /dev/null +++ b/.github/workflows/pr-body-check.yml @@ -0,0 +1,50 @@ +name: PR body check + +# Fails the check when a pull request description is missing the sections from +# .github/PULL_REQUEST_TEMPLATE.md. GitHub only auto-fills that template in the +# web "compose" form and in `gh pr create` with no --body; a PR opened through +# the REST API or `gh pr create --body "..."` (the path most AI tools take) +# silently skips it. This check is the only thing that actually enforces it. +# +# Recovery is automatic: editing the PR description fires the `edited` event and +# re-runs this check with the new body. No new commit or manual re-run needed. + +on: + pull_request: + types: [opened, edited, reopened, synchronize] + branches: [main] + +permissions: + contents: read + +jobs: + check: + runs-on: ubuntu-latest + steps: + - name: Check required sections are present + env: + PR_BODY: ${{ github.event.pull_request.body }} + run: | + set -euo pipefail + required=( + "## Summary" + "## How to review" + "## Secrets hygiene checklist" + ) + missing=() + for section in "${required[@]}"; do + if ! printf '%s' "$PR_BODY" | grep -qiF "$section"; then + missing+=("$section") + fi + done + if [ ${#missing[@]} -ne 0 ]; then + echo "::error::Your PR description is missing required sections. Start from the template (.github/PULL_REQUEST_TEMPLATE.md) and keep these headings:" + for m in "${missing[@]}"; do echo " - $m"; done + echo "" + echo "If you (or an AI tool) opened this PR without the template, click 'Edit' on the" + echo "PR description, paste the template, and fill it in. Editing the description" + echo "re-runs this check automatically. A complete PR is easy to review and" + echo "reproducible: that is part of the assignment." + exit 1 + fi + echo "All required PR sections present." diff --git a/.gitignore b/.gitignore index 2b76d7c..794e281 100644 --- a/.gitignore +++ b/.gitignore @@ -6,6 +6,13 @@ Thumbs.db # hyf .hyf/score.json +# dbt / Databricks secrets +profiles.yml +task-2/profiles.yml +dbt_packages/ +target/ +logs/ + # Editor and IDE settings .vscode/ .idea/ diff --git a/.hyf/grader_lib.sh b/.hyf/grader_lib.sh new file mode 100755 index 0000000..3142cfe --- /dev/null +++ b/.hyf/grader_lib.sh @@ -0,0 +1,262 @@ +#!/usr/bin/env bash +# grader_lib.sh — shared helpers for HYF Data Track autograders. +# Source this at the top of test.sh: +# source "$(dirname "$0")/grader_lib.sh" +# +# Provides: pass(), fail(), warn(), blocker(), print_results(), write_score(), +# and a set of common static-analysis checks derived from recurring +# PR review patterns across cohort c55. +# +# blocker(): use for leaked-secret findings (a committed profiles.yml/.env, +# a hardcoded password/connection string). It behaves like fail() for the +# printed report, but also flips a flag that forces write_score() to report +# pass=false regardless of the earned point total -- a leaked secret must +# be fixed before the PR can pass, it cannot be "pointed around." + +_grader_details=() +_grader_blocker=false + +pass() { _grader_details+=("✓ PASS $1"); } +fail() { _grader_details+=("✗ FAIL $1"); } +warn() { _grader_details+=("⚠ WARN $1"); } +blocker() { _grader_details+=("🚫 BLOCKER $1"); _grader_blocker=true; } + +print_results() { + local header="${1:-Autograder Results}" + echo "" + echo "=== $header ===" + for line in "${_grader_details[@]}"; do echo " $line"; done + echo "" +} + +write_score() { + # write_score [] + local score="$1" + local passing="$2" + local outfile="${3:-$(dirname "${BASH_SOURCE[0]}")/score.json}" + local pass_flag="false" + [[ "$score" -ge "$passing" ]] && pass_flag="true" + if [[ "$_grader_blocker" == true ]]; then + pass_flag="false" + echo "🚫 A blocker was found (leaked secret) -- forcing pass=false regardless of score." >&2 + fi + cat > "$outfile" << JSON +{ + "score": $score, + "pass": $pass_flag, + "passingScore": $passing +} +JSON + echo "Score: $score / 100 (passing: $passing) pass=$pass_flag" +} + +# ── Common static-analysis checks ──────────────────────────────────────────── +# Each function: returns 0 on pass, 1 on fail/warn (for caller logic). +# All feedback goes through pass()/fail()/warn() so it appears in print_results. + +check_no_print_statements() { + # Usage: check_no_print_statements [label] + # Flags bare print() calls that should be logging calls. + local dir="${1:-.}" + local label="${2:-$dir}" + local found + found=$(grep -rn "^[[:space:]]*print(" "$dir" --include="*.py" 2>/dev/null | grep -v "# noqa" || true) + if [[ -n "$found" ]]; then + local count + count=$(echo "$found" | wc -l | tr -d ' ') + warn "$label: $count print() call(s) found — use logging.info/warning/error instead (see Week 1 Ch1)" + return 1 + fi + return 0 +} + +check_no_notimplemented() { + # Usage: check_no_notimplemented [label] + # Flags NotImplementedError stubs left in after implementation. + local dir="${1:-.}" + local label="${2:-$dir}" + local found + found=$(grep -rn "raise NotImplementedError" "$dir" --include="*.py" 2>/dev/null || true) + if [[ -n "$found" ]]; then + fail "$label: raise NotImplementedError still present — remove stubs before submitting" + return 1 + fi + return 0 +} + +check_no_relative_imports() { + # Usage: check_no_relative_imports [label] + # Flags `from .module import x` in scripts not inside a proper package. + # Relative imports break the grader: python3 src/cleaner.py fails with + # "attempted relative import with no known parent package". + local dir="${1:-.}" + local label="${2:-$dir}" + local found + found=$(grep -rn "^from \." "$dir" --include="*.py" 2>/dev/null || true) + if [[ -n "$found" ]]; then + fail "$label: relative import found (from .module) — use absolute: 'from src.module import x'" + return 1 + fi + return 0 +} + +check_no_logging_in_utils() { + # Usage: check_no_logging_in_utils + # utils.py should be pure helpers; logging config belongs in the entry point. + local file="${1:-task-1/src/utils.py}" + if [[ ! -f "$file" ]]; then return 0; fi + if grep -qE "logging\.basicConfig|logging\.getLogger" "$file"; then + warn "$file: logging.basicConfig/getLogger found — logging setup belongs in cleaner.py or the entry-point, not in utils" + return 1 + fi + return 0 +} + +check_gitignore_python() { + # Usage: check_gitignore_python [] + # Warns when Python cache patterns are absent from .gitignore. + local gi="${1:-.gitignore}" + if [[ ! -f "$gi" ]]; then + warn ".gitignore is missing — add one so __pycache__/ and *.pyc are not committed" + return 1 + fi + local ok=true + if ! grep -q "__pycache__" "$gi"; then + warn ".gitignore missing __pycache__/ — Python bytecode cache dirs should not be committed" + ok=false + fi + if ! grep -qE "^\*\.pyc$|^.*\*\.pyc" "$gi"; then + warn ".gitignore missing *.pyc — compiled Python files should not be committed" + ok=false + fi + if ! grep -qE "^\.env$|^\.env\b" "$gi"; then + warn ".gitignore missing .env — secret files should not be committed" + ok=false + fi + if [[ "$ok" = true ]]; then pass ".gitignore correctly excludes __pycache__/, *.pyc, and .env"; fi +} + +check_screenshot_is_png() { + # Usage: check_screenshot_is_png [] + # Awards full credit for .png, warns (and still credits) for .jpg/.jpeg, + # zero for missing. Matches the pattern flagged in c55 PR reviews. + local expected_png="$1" + local dir + dir="$(dirname "$expected_png")" + local base + base="$(basename "$expected_png" .png)" + + if [[ -s "$expected_png" ]]; then + pass "screenshot is $expected_png (.png format ✓)" + return 0 + fi + for ext in jpg jpeg; do + if [[ -s "$dir/$base.$ext" ]]; then + warn "screenshot is .$ext but should be .png — rename to $base.png (partial credit still given)" + return 1 + fi + done + fail "screenshot missing: $expected_png not found" + return 2 +} + +check_silent_zero_in_except() { + # Usage: check_silent_zero_in_except + # Detects the pattern: try: x = compute() / except: x = 0 + # which silently corrupts data instead of skipping or raising. + local file="$1" + if [[ ! -f "$file" ]]; then return 0; fi + local found + found=$(python3 - "$file" 2>/dev/null << 'PY' +import ast, sys +try: + tree = ast.parse(open(sys.argv[1]).read()) +except SyntaxError: + sys.exit(0) +for node in ast.walk(tree): + if isinstance(node, ast.ExceptHandler): + for stmt in node.body: + if isinstance(stmt, ast.Assign): + if isinstance(stmt.value, ast.Constant) and stmt.value.value == 0: + print(f"line {stmt.lineno}: '{ast.unparse(stmt)}' — sets field to 0 in except block (silent data corruption)") +PY +) + if [[ -n "$found" ]]; then + warn "$file: silent 0-assignment in except block — skip the row or raise instead of setting to 0:\n $found" + return 1 + fi + return 0 +} + +check_exception_logged() { + # Usage: check_exception_logged + # Warns when except blocks log/print a message but don't include the + # exception variable (e, err, exc), meaning the error type is lost. + local dir="${1:-.}" + local found + found=$(python3 - "$dir" 2>/dev/null << 'PY' +import ast, os, sys +issues = [] +for root, _, files in os.walk(sys.argv[1]): + for fname in files: + if not fname.endswith(".py"): + continue + path = os.path.join(root, fname) + try: + tree = ast.parse(open(path).read()) + except SyntaxError: + continue + for node in ast.walk(tree): + if not isinstance(node, ast.ExceptHandler): + continue + exc_var = node.name # e.g. "e" in `except ValueError as e` + if not exc_var: + continue + for stmt in node.body: + for call in ast.walk(stmt): + if not isinstance(call, ast.Call): + continue + # Is it a logging.* or print call? + func = call.func + is_log = (isinstance(func, ast.Attribute) and + isinstance(func.value, ast.Name) and + func.value.id == "logging") + is_print = isinstance(func, ast.Name) and func.id == "print" + if not (is_log or is_print): + continue + # Does the call reference the exception variable? + src = ast.unparse(call) + if exc_var not in src: + issues.append(f"{path}:{call.lineno}: log message doesn't include exception variable '{exc_var}' — add it for easier debugging") +if issues: + for i in issues[:3]: # cap at 3 to keep output readable + print(i) +PY +) + if [[ -n "$found" ]]; then + warn "exception variable not included in log message (harder to debug):\n $found" + return 1 + fi + return 0 +} + +check_ruff() { + # Usage: check_ruff [