Skip to content

Hannah#15

Open
hannahwn wants to merge 4 commits into
HackYourAssignment:mainfrom
hannahwn:main
Open

Hannah#15
hannahwn wants to merge 4 commits into
HackYourAssignment:mainfrom
hannahwn:main

Conversation

@hannahwn

@hannahwn hannahwn commented Jun 4, 2026

Copy link
Copy Markdown

No description provided.

hannahwn added 2 commits June 2, 2026 21:13
@hannahwn
Merge pull request #1 from HackYourAssignment/main
7afd7d4 
Sync from HackYourAssignment/c55-data-week-5 (Grade workflow fix)
@hannahwn
trial of CI pipeline
45b18fc
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown

✅ Image pushed to ACR

hyfregistry.azurecr.io/hannahwn-pipeline:45b18fcc061d5e99ab15c60548ccf7e703e8f4d8

Browse the registry in the Azure Portal: https://portal.azure.com/#@hackyourfuture.nl/resource/subscriptions/1120c89d-2a5f-4a15-a582-2ea34f0bb5c3/resourceGroups/rg-hyf-data/providers/Microsoft.ContainerRegistry/registries/hyfregistry/repository

Task 7 verification: your Dockerfile built and pushed cleanly from commit 45b18fc.

@github-actions

This comment has been minimized.

Sign in to view
@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown

✅ Image pushed to ACR

hyfregistry.azurecr.io/hannahwn-pipeline:68cd0e015c72206fbccf58c97eb5f8b0184bbc8f

Browse the registry in the Azure Portal: https://portal.azure.com/#@hackyourfuture.nl/resource/subscriptions/1120c89d-2a5f-4a15-a582-2ea34f0bb5c3/resourceGroups/rg-hyf-data/providers/Microsoft.ContainerRegistry/registries/hyfregistry/repository

Task 7 verification: your Dockerfile built and pushed cleanly from commit 68cd0e0.

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown

📝 HackYourFuture auto grade

Assignment Score: 83 / 100 ✅

Status: ✅ Passed
Minimum score to pass: 60
🧪 The auto grade is experimental and still being improved

Test Details 

=== Week 5 Autograder ===
  PASS: Level 1: required files (15/15 pts)
  PASS: Dockerfile uses python:3.11-slim base image
  PASS: Dockerfile copies requirements before source (cache-friendly)
  PASS: Dockerfile has a CMD instruction
  PASS: Level 2: Dockerfile (15/15 pts)
  PASS: tests/test_pipeline.py has 11 test functions (≥2 required)
  PASS: tests/test_pipeline.py has no NotImplementedError stubs remaining
  PASS: Level 3: unit tests (10/10 pts)
  PASS: requirements.txt has 6 pinned package(s)
  PASS: requirements.txt pins satisfied (no uv.lock needed)
  PASS: Level 4: pinned dependencies (10/10 pts)
  PASS: ci.yml triggers on pull_request
  PASS: ci.yml triggers on push to main
  PASS: ci.yml runs ruff check (lint)
  PASS: ci.yml runs ruff format (format check)
  PASS: ci.yml runs pytest
  PASS: ci.yml runs docker build
  PASS: Level 5: CI workflow (20/20 pts)
  PASS: pipeline.py reads config from os.environ/os.getenv
  FAIL: pipeline.py still contains NotImplementedError
  PASS: Level 6: env-var config (10/15 pts)
  FAIL: assets/acr_push_week5.png missing (Task 7 deliverable)
  PASS: Level 7: ACR screenshot (0/10 pts)
  PASS: AI_ASSIST.md has all three required sections
  FAIL: AI_ASSIST.md still contains TODO placeholders or is too short (1992 chars)
  PASS: Level 8: AI report (3/5 pts)
  PASS: .gitignore correctly excludes __pycache__/, *.pyc, and .env

Score: 83 / 100  (passing: 60)  pass=true

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown

✅ Image pushed to ACR

hyfregistry.azurecr.io/hannahwn-pipeline:4b02355b5b2632904ae02096e63f5bdab534f5d9

Browse the registry in the Azure Portal: https://portal.azure.com/#@hackyourfuture.nl/resource/subscriptions/1120c89d-2a5f-4a15-a582-2ea34f0bb5c3/resourceGroups/rg-hyf-data/providers/Microsoft.ContainerRegistry/registries/hyfregistry/repository

Task 7 verification: your Dockerfile built and pushed cleanly from commit 4b02355.

qiraahmad
qiraahmad reviewed Jun 14, 2026
View reviewed changes
Comment thread src/pipeline.py
enriched = join_customers(sales_clean, customers_raw)

reports = build_reports(enriched)
write_outputs(reports, OUTPUT_DIR)

@qiraahmad qiraahmad Jun 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

OUTPUT_DIR and GITHUB_USERNAME are not defined anywhere, this will cause an error.

You already have output_dir defined, use that instead

qiraahmad
qiraahmad reviewed Jun 14, 2026
View reviewed changes
Comment thread src/pipeline.py
@@ -55,6 +81,22 @@ def run() -> None:
save_results(records, output_dir)
logger.info("pipeline complete")

@qiraahmad qiraahmad Jun 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

add this in the end

qiraahmad
qiraahmad reviewed Jun 14, 2026
View reviewed changes
Comment thread src/pipeline.py
import logging
import os
from pathlib import Path
DATA_DIR = Path("data")

@qiraahmad qiraahmad Jun 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Imports must come before other module-level code — ruff check src flags this as E402. Move the from src... imports (lines 16–19) up with the other imports at the top, before DATA_DIR = Path("data").

Run ruff check src locally before pushing as there are several lint errors that would fail your CI lint step.

qiraahmad
qiraahmad reviewed Jun 14, 2026
View reviewed changes
Comment thread .github/workflows/ci.yml
- name: Build image
run: echo "TODO implement this step"
run: docker build -t hannahwn-pipeline:${{ github.sha }} .
- name: Azure login

@qiraahmad qiraahmad Jun 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

These Azure login/push steps aren't needed as GitHub blocks secrets on pull_request workflows from forks.

Please remove lines 39–48 and keep only: checkout → install → lint → format → test → docker build.

qiraahmad
qiraahmad reviewed Jun 14, 2026
View reviewed changes
Comment thread tests/test_pipeline.py
assert len(content.strip().splitlines()) >= 2


class TestDownloadInputs:

@qiraahmad qiraahmad Jun 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This test calls real Azure Blob Storage and fails without credentials (ClientAuthenticationError). CI runs pytest -q with no Azure login, so this breaks the pipeline. You can remove this test or mock the Azure client with monkeypatch so the test doesn't need real credentials.

qiraahmad
qiraahmad reviewed Jun 14, 2026
View reviewed changes
Comment thread assets/azureportalproblem.png

@qiraahmad qiraahmad Jun 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

To fix:

  1. Log in with your HYF account: az login --use-device-code --tenant 07a14c4e-d88c-42f7-83b3-13af7e57ff3d
  2. Push your image locally and take a screenshot of the successful push
  3. Save it as assets/acr_push_week5.png (exact filename required by the autograder)

@qiraahmad

qiraahmad commented Jun 14, 2026 •
edited
Loading

Copy link
Copy Markdown

Grade: 3/5

Good effort! The core Week 5 functions are implemented well. The Dockerfile is correct, dependencies are pinned, and CI includes the required lint/test/build steps.

However, the submission doesn't work cleanly end-to-end yet, you can check comments for more

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@qiraahmad qiraahmad qiraahmad left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@hannahwn @qiraahmad