Reuse the cache created for latest `main` on PRs/branches if `setup.py` is not modified #25445

ydshieh · 2023-08-10T17:27:29Z

What does this PR do?

For a PR or a branch, if setup.py is not modified (compare to the common ancestor with the main), let's use cache that is created for the setup.py from the latest commit on the main branch.

latest means the latest one on main at the moment where a run is triggered.

Motivation:

avoid having cache for main and pull (for most cases), which is introduced in Separate CircleCI cache between main and pull or other branches #24886 to avoid unexpected/undesired edge cases.
avoid the cache creation and store for PRs/branches that still have (very) old setup.py as they don't rebase on more recent main

With this PR, we expect the storage usage for cache could be reduced dramatically 🤞 .

The artifact in this job run shows generated_config.txt has explicit checksum used v0.7-pipelines_torch-main-pip-9RXs1YQ8L2beP4cdAfRDkWX0VRTtWaQodDVKzvyJwPI= rather than {{ checksum "setup.py" }}

.circleci/create_circleci_config.py

HuggingFaceDocBuilderDev · 2023-08-10T17:57:33Z

The documentation is not available anymore as the PR was closed or merged.

ydshieh · 2023-08-10T21:47:02Z

src/transformers/models/bert/modeling_bert.py

@@ -884,6 +884,8 @@ class BertModel(BertPreTrainedModel):
    `add_cross_attention` set to `True`; an `encoder_hidden_states` is then expected as an input to the forward pass.
    """

+    a = 3


I will remove this before merge

.circleci/create_circleci_config.py

sgugger

Thanks for working on this! Let's try it for a bit and see if it impacts any workflow negatively.

ydshieh · 2023-08-11T10:14:10Z

.circleci/config.yml

            - store_artifacts:
-                  path: ~/transformers/tests_fetched_summary.txt
+                  path: test_preparation/tests_fetched_summary.txt


need this in create_circleci_config.py to check if tests is in the list

ydshieh · 2023-08-11T10:16:18Z

.circleci/create_circleci_config.py

+        summary_file = os.path.join(folder, "tests_fetched_summary.txt")
+        if os.path.exists(summary_file):
+            with open(summary_file) as f:
+                tests_fetched_summary = f.read()
+                setup_file_modifiled = "### TEST TO RUN ###\n- tests\n" in tests_fetched_summary


We need to check the summary file instead of filtered_test_list as it is already changed to a list in filter_tests method

if test_files == ["tests"]: test_files = [os.path.join("tests", f) for f in os.listdir("tests") if f not in ["__init__.py"] + filters]

ydshieh · 2023-08-11T10:19:11Z

@sgugger FYI: I need to change 2 more places to make it work correctly for all cases. See my last 2 review comments.

sgugger

Works for me!

ydshieh · 2023-08-11T15:55:28Z

Hi @sgugger

I am afraid I have to revert this PR until we do something to enabling sharing cache (see below at the end). From this section

PRs from the same fork repo share a cache (this includes, as previously stated, that PRs in the main repo share a cache with main).
Two PRs in different fork repos have different caches.

The cache is never shared between PRs/branches from different forks. This might explains the question I posted about why the same cache key could be found sometimes but not other times.

The PR description of #24886 is partially valid as lhoestq created a branch in transformers rather than a PR from his own forked repo.

The way to share cache is

Enabling the sharing of environment variables allows cache sharing between the original repo and all forked builds.

But we need to be careful if we have sensitive env. variables or not.

…`setup.py` is not modified (#25445)" This reverts commit 1d75768.

…25466) Revert "Reuse the cache created for latest `main` on PRs/branches if `setup.py` is not modified (#25445)" This reverts commit 1d75768.

ydshieh commented Aug 10, 2023

View reviewed changes

.circleci/create_circleci_config.py Outdated Show resolved Hide resolved

ydshieh force-pushed the rework_cache_again branch from 631f5b2 to 98772c4 Compare August 10, 2023 18:05

ydshieh commented Aug 10, 2023

View reviewed changes

.circleci/create_circleci_config.py Show resolved Hide resolved

ydshieh commented Aug 10, 2023

View reviewed changes

.circleci/create_circleci_config.py Show resolved Hide resolved

ydshieh changed the title ~~[WIP] Rework cache again~~ Rework cache again Aug 10, 2023

ydshieh marked this pull request as ready for review August 10, 2023 21:50

ydshieh changed the title ~~Rework cache again~~ Reuse the cache created for main on PRs/branches if setup.py not modified Aug 10, 2023

ydshieh changed the title ~~Reuse the cache created for main on PRs/branches if setup.py not modified~~ Reuse the cache created for latest main on PRs/branches if setup.py not modified Aug 10, 2023

ydshieh changed the title ~~Reuse the cache created for latest main on PRs/branches if setup.py not modified~~ Reuse the cache created for latest main on PRs/branches if setup.py is not modified Aug 10, 2023

ydshieh commented Aug 10, 2023

View reviewed changes

.circleci/create_circleci_config.py Show resolved Hide resolved

ydshieh requested a review from sgugger August 10, 2023 22:16

sgugger approved these changes Aug 11, 2023

View reviewed changes

fix

c56762e

ydshieh force-pushed the rework_cache_again branch from 47334f8 to c56762e Compare August 11, 2023 08:10

ydshieh added 2 commits August 11, 2023 11:35

fix

35ac98f

fix

0ba080b

ydshieh commented Aug 11, 2023

View reviewed changes

sgugger approved these changes Aug 11, 2023

View reviewed changes

ydshieh merged commit 1d75768 into main Aug 11, 2023

ydshieh deleted the rework_cache_again branch August 11, 2023 12:40

ydshieh added a commit that referenced this pull request Aug 11, 2023

Revert "Reuse the cache created for latest main on PRs/branches if …

158c646

…`setup.py` is not modified (#25445)" This reverts commit 1d75768.

ydshieh mentioned this pull request Aug 11, 2023

Revert "Reuse the cache created for latest main on PRs/branches if setup.py is not modified" #25466

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reuse the cache created for latest `main` on PRs/branches if `setup.py` is not modified #25445

Reuse the cache created for latest `main` on PRs/branches if `setup.py` is not modified #25445

ydshieh commented Aug 10, 2023 •

edited

Loading

HuggingFaceDocBuilderDev commented Aug 10, 2023 •

edited

Loading

ydshieh Aug 10, 2023

sgugger left a comment

ydshieh Aug 11, 2023

ydshieh Aug 11, 2023

ydshieh commented Aug 11, 2023

sgugger left a comment

ydshieh commented Aug 11, 2023 •

edited

Loading

Reuse the cache created for latest main on PRs/branches if setup.py is not modified #25445

Reuse the cache created for latest main on PRs/branches if setup.py is not modified #25445

Conversation

ydshieh commented Aug 10, 2023 • edited Loading

What does this PR do?

HuggingFaceDocBuilderDev commented Aug 10, 2023 • edited Loading

ydshieh Aug 10, 2023

Choose a reason for hiding this comment

sgugger left a comment

Choose a reason for hiding this comment

ydshieh Aug 11, 2023

Choose a reason for hiding this comment

ydshieh Aug 11, 2023

Choose a reason for hiding this comment

ydshieh commented Aug 11, 2023

sgugger left a comment

Choose a reason for hiding this comment

ydshieh commented Aug 11, 2023 • edited Loading

Reuse the cache created for latest `main` on PRs/branches if `setup.py` is not modified #25445

Reuse the cache created for latest `main` on PRs/branches if `setup.py` is not modified #25445

ydshieh commented Aug 10, 2023 •

edited

Loading

HuggingFaceDocBuilderDev commented Aug 10, 2023 •

edited

Loading

ydshieh commented Aug 11, 2023 •

edited

Loading