Mohammed A#5
Conversation
This comment has been minimized.
This comment has been minimized.
mohammedalfakih-dev
changed the title
This comment has been minimized.
This comment has been minimized.
GitHub withholds AZURE_CREDENTIALS from pull_request runs that originate from a fork (security default), which made the Azure login step fail with "client-id and tenant-id are supplied". Guarding the Azure login, ACR login, and image push with `if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.fork == false` skips them on fork PRs so CI stays green, while still running them on pushes to main and on PRs opened inside the assignment repo itself. The Task 7 ACR-push deliverable (assets/acr_push_week5.png) is unaffected since it is produced from a local push, not from CI.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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 |
…-week-5 into week5/mohammed-alfakih # Conflicts: # .github/workflows/ci.yml
📝 HackYourFuture auto gradeAssignment Score: 100 / 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 |
AgneseGi
left a comment
AgneseGi
left a comment
There was a problem hiding this comment.
Technically strong, but less aligned with the Week 5 starter. The pipeline now requires GITHUB_USERNAME instead of the expected API_KEY, so the README Docker command would fail. It also depends on Azure/data access by default, which makes the container less reproducible.
| def get_config() -> dict: | ||
| """ | ||
| Return configuration read from environment variables. | ||
| github_username = os.getenv("GITHUB_USERNAME") |
There was a problem hiding this comment.
This changes the assignment contract from API_KEY to GITHUB_USERNAME. The Week 5 starter and README expect the container to run with something like docker run --rm -e API_KEY=test ..., but this implementation would fail unless GITHUB_USERNAME is provided
| raise NotImplementedError("Task 1: return at least one sample record") | ||
| logger.info("Starting Week 4 Pandas pipeline") | ||
|
|
||
| download_inputs(data_dir) |
There was a problem hiding this comment.
Calling download_inputs() unconditionally means the container depends on Azure access by default. For this assignment, the pipeline should be easy to run in a container with simple environment config. You could eg make the Azure download optional or supporting local/sample input data for reproducible Docker runs
| <!-- Paste the exact prompt you gave to an LLM (ChatGPT, Claude, Copilot, etc.). --> | ||
|
|
||
| TODO: paste your prompt here. | ||
| <!-- After building the Docker image successfully, I received authentication errors when running the container. The pipeline worked locally, but inside Docker Azure authentication failed with DefaultAzureCredential. why Docker could not access the Azure data --> |
There was a problem hiding this comment.
This answer is still inside an HTML comment, so it is hidden in rendered Markdown
| ChatGPT first suggested using local sample CSV files instead of downloading data from Azure Blob Storage. | ||
|
|
||
| ```python | ||
| # TODO: paste the AI-generated code here |
There was a problem hiding this comment.
There is still a TODO placeholder inside the code block
3 participants
Summary