Skip to content

Fix dependency issue#2810

Open
smcmurtry wants to merge 2 commits intomainfrom
fix/dependency-issue
Open

Fix dependency issue#2810
smcmurtry wants to merge 2 commits intomainfrom
fix/dependency-issue

Conversation

@smcmurtry
Copy link
Copy Markdown
Contributor

@smcmurtry smcmurtry commented Mar 25, 2026

Summary | Résumé

Fix dependency issue - builds have failed since monday, this should fix the build error: https://github.com/cds-snc/notification-api/actions/runs/23490450868/job/68547058126

I tested locally with:

docker build \
  --build-arg GIT_SHA=$(git rev-parse --short HEAD) \
  -f ci/Dockerfile.lambda \
  -t api-lambda:test \
  .

And it built successfully.

Release Instructions | Instructions pour le déploiement

None.

Reviewer checklist | Liste de vérification du réviseur

  • This PR does not break existing functionality.
  • This PR does not violate GCNotify's privacy policies.
  • This PR does not raise new security concerns. Refer to our GC Notify Risk Register document on our Google drive.
  • This PR does not significantly alter performance.
  • Additional required documentation resulting of these changes is covered (such as the README, setup instructions, a related ADR or the technical documentation).

⚠ If boxes cannot be checked off before merging the PR, they should be moved to the "Release Instructions" section with appropriate steps required to verify before release. For example, changes to celery code may require tests on staging to verify that performance has not been affected.

@smcmurtry smcmurtry requested a review from jimleroyer as a code owner March 25, 2026 14:52
Copilot AI review requested due to automatic review settings March 25, 2026 14:52
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to fix CI/docker build failures for the Lambda image by adjusting how Poetry dependencies and Lambda runtime dependencies are installed.

Changes:

  • Copies README.md alongside pyproject.toml/poetry.lock during the dependency-install layer.
  • Replaces poetry add awslambdaric wheel with a direct pip install awslambdaric during image build.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 23 to 28
COPY pyproject.toml poetry.lock README.md ${TASK_ROOT}/

# Install Poetry dependencies first (including setuptools and wheel)
RUN python -m venv ${APP_VENV} \
&& . ${APP_VENV}/bin/activate \
&& poetry install \
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

Including README.md in the dependency-layer COPY means any README change will invalidate Docker cache and force a full dependency reinstall. If the only reason for copying it is to satisfy readme = "README.md" during poetry install, consider switching this step to poetry install --no-root (or otherwise avoiding installing the project package in this layer) so the deps layer can stay independent of README edits.

Suggested change
COPY pyproject.toml poetry.lock README.md ${TASK_ROOT}/
# Install Poetry dependencies first (including setuptools and wheel)
RUN python -m venv ${APP_VENV} \
&& . ${APP_VENV}/bin/activate \
&& poetry install \
COPY pyproject.toml poetry.lock ${TASK_ROOT}/
# Install Poetry dependencies first (including setuptools and wheel)
RUN python -m venv ${APP_VENV} \
&& . ${APP_VENV}/bin/activate \
&& poetry install --no-root \

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants