Mareh A.#13
Conversation
Sync from HackYourAssignment/c55-data-week-5 (Grade workflow fix)
This comment has been minimized.
This comment has been minimized.
|
✅ Image pushed to ACR
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 |
This comment has been minimized.
This comment has been minimized.
|
✅ Image pushed to ACR
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 |
GitHub withholds AZURE_CREDENTIALS from pull_request runs from forks, causing azure/login to fail. The if-guard skips the Azure/ACR steps on fork PRs so CI stays green; the central Grade ACR push workflow on main handles the actual push.
📝 HackYourFuture auto gradeAssignment Score: 93 / 100 ✅Status: ✅ Passed Test Details |
|
✅ Image pushed to ACR
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 |
|
Hi, I'm not the reviewer on this PR, but I don't think it's ok to skip the AI assist documentation. I would consider it in the learning process to be as essential as "where did your data come from" (in the data process). |
| # ruff== | ||
| # | ||
| # Add your pinned dependencies below: | ||
| azure-storage-blob |
There was a problem hiding this comment.
you forgot to add a version against some of the packages
| output_dir = Path(config["output_dir"]) | ||
| save_results(records, output_dir) | ||
| logger.info("pipeline complete") | ||
| download_inputs(DATA_DIR) |
There was a problem hiding this comment.
Week 5's starter run() only needs: config(), fetch(), save() and the final log, after that you run the full Week 4 pipeline, which needs ACCOUNT_URL and SOURCE_CONTAINER. docker run will fail at download_inputs() because the image doesn't include data/ and has no Azure credentials.
Either stop after save_results for Week 5, or COPY data/ into the Dockerfile
| from src.transform import join_customers | ||
| from src.report import build_reports, write_outputs | ||
|
|
||
| GITHUB_USERNAME = "mareh-aboghanem" |
There was a problem hiding this comment.
better to use os.getenv("GITHUB_USERNAME")
| from src.pipeline import fetch_data, get_config, save_results | ||
|
|
||
| import pandas as pd | ||
| from src.pipeline import fetch_data, get_config, save_results, clean_sales |
There was a problem hiding this comment.
Import clean_sales from src.clean, not src.pipeline. It only works now because pipeline happens to import it at module level.
| COPY requirements.txt . | ||
| # TODO: install dependencies | ||
|
|
||
| RUN pip install -r requirements.txt |
There was a problem hiding this comment.
Please test docker run after docker build as the image currently only copies src/, not data/.
- RUN pip install --no-cache-dir -r requirements.txt (smaller image)
- COPY data/ ./data/ (your run() reads from Path("data"); without this, the container has no CSV files and no Azure login to download them)
Without this, the run will fail at download_inputs(). Try: docker run --rm -e API_KEY=test
|
Good job, especially with the CI with fork guards and Week 4 continuity. However, AI_ASSIST.md completely unfilled Task 8 not submitted, and look into the comments above. 3/5 |
3 participants
I have completed and verified all core pipeline, testing, and Azure container deployment tasks, but did not have time to finish task 8, the AI assistant part due to the submission deadline.