Skip to content

ci: Cache .cache/pip instead of .venv #36

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 20, 2025
Merged

Conversation

fischeti
Copy link
Collaborator

@fischeti fischeti commented May 19, 2025

  • Merges init-deps and init-pd CI jobs, which essentially did the same thing.
  • Caching the entire virtual environment causes some spurious issues in the CI. Instead, only the pip packages are now cached, similar as in the snitch cluster.

@fischeti fischeti force-pushed the ci-disable-venv-caching branch from 6a2f642 to fcafaac Compare May 19, 2025 12:14
@fischeti fischeti force-pushed the ci-disable-venv-caching branch from fcafaac to 4713e80 Compare May 19, 2025 15:17
@fischeti fischeti requested review from Copilot and colluca May 19, 2025 16:05
@fischeti fischeti marked this pull request as ready for review May 19, 2025 16:05
Copy link
Contributor

@Copilot 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 updates the CI caching strategy to cache only pip packages instead of the entire virtual environment and consolidates the init-deps and init-pd jobs into a single job.

  • Updated the Makefile to use a new PIP_CACHE_DIR variable for pip package installation.
  • Modified .gitlab/common.yml to cache only the pip packages and removed caching of the virtual environment.
  • Updated .gitlab-ci.yml to replace references to the removed init-pd job with init-deps and added artifacts for pd/ci.yml.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
Makefile Introduced PIP_CACHE_DIR and updated pip install commands to use caching.
.gitlab/common.yml Replaced caching of .venv with .cache/pip and centralized cache settings.
.gitlab-ci.yml Updated job dependencies and artifacts, replacing the init-pd reference.
Comments suppressed due to low confidence (1)

.gitlab/common.yml:6

  • [nitpick] Verify that the use of the YAML merge key (<<: *global_cache) is supported by the GitLab runner version in use and is clearly documented.
PIP_CACHE_DIR: "$CI_PROJECT_DIR/.cache/pip"

@@ -154,6 +154,7 @@ include $(PB_ROOT)/target/sim/traces.mk
########

BASE_PYTHON ?= python
PIP_CACHE_DIR ?= $(PB_ROOT)/.cache/pip
Copy link
Preview

Copilot AI May 19, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider adding a brief comment explaining the purpose of PIP_CACHE_DIR to improve clarity for future maintainers.

Copilot uses AI. Check for mistakes.

subpipe:
stage: build
needs:
- init-pd
- init-deps
Copy link
Preview

Copilot AI May 19, 2025

Choose a reason for hiding this comment

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

Ensure that all downstream jobs and documentation reflect the change from init-pd to init-deps to avoid potential job dependency issues.

Copilot uses AI. Check for mistakes.

Copy link
Contributor

@colluca colluca left a comment

Choose a reason for hiding this comment

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

LGTM

@fischeti fischeti merged commit 7a6620a into main May 20, 2025
5 checks passed
@fischeti fischeti deleted the ci-disable-venv-caching branch May 20, 2025 07:40
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