zozo123 commented on code in PR #73182:
URL: https://github.com/apache/airflow/pull/73182#discussion_r4028088696
##########
.github/actions/install-prek/action.yml:
##########
@@ -143,16 +137,67 @@ runs:
shell: bash
run: cat ~/.cache/prek/prek.log || true
if: always()
+ - name: "Decide whether to refresh prek cache"
+ id: cache-policy
+ shell: bash
+ env:
+ EVENT_NAME: ${{ github.event_name }}
+ SAVE_CACHE: ${{ inputs.save-cache }}
+ STASH_HIT: ${{ steps.restore-prek-cache.outputs.stash-hit }}
+ TAR_RESTORED: ${{ steps.restore-prek-tar.outputs.tar-restored }}
+ run: |
+ SAVE=false
+ if [[ "${SAVE_CACHE}" == "true" ]]; then
+ if [[ "${STASH_HIT}" != "true" || "${TAR_RESTORED}" != "true" || \
Review Comment:
Addressed in 94c15a9bc8. The install step now snapshots prek 0.5.2 hook/repo
markers before the first attempt and after eventual success, across the full
retry sequence. Marker changes or uncertain comparison force an eligible writer
save. The lifecycle test proves repair → save → restore → healthy reuse/no
save, and the real prek 0.5.2 test proves marker removal causes replacement and
the repaired cache is subsequently reused.
##########
scripts/ci/prek_cache_key.py:
##########
@@ -0,0 +1,71 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Compute a cache identity for prek's host-side hook environments."""
+
+from __future__ import annotations
+
+import hashlib
+import json
+import os
+import platform
+import sys
+import sysconfig
+from collections.abc import Mapping
+from pathlib import Path
+
+REQUIRED_INPUTS = ("PLATFORM", "UV_VERSION", "PREK_VERSION",
"PREK_CONFIG_HASH", "GITHUB_WORKSPACE")
+
+
+def build_cache_identity(environ: Mapping[str, str]) -> dict[str, str]:
+ """Do not reuse virtualenvs across incompatible hosts or absolute
interpreter paths."""
+ missing = [name for name in REQUIRED_INPUTS if not environ.get(name)]
+ if missing:
+ raise ValueError(f"Missing prek cache inputs: {', '.join(missing)}")
+ try:
+ os_release = platform.freedesktop_os_release()
+ except OSError:
+ os_release = {}
+ return {
+ **{name: environ[name] for name in REQUIRED_INPUTS},
+ "system": platform.system(),
+ "machine": platform.machine(),
+ "os_id": os_release.get("ID", ""),
+ "os_version": os_release.get("VERSION_ID", ""),
+ "python_version": platform.python_version(),
+ "python_abi": sysconfig.get_config_var("SOABI") or "",
+ "python_executable": str(Path(sys.executable).resolve()),
Review Comment:
Addressed by removing the OS/ABI/home/path identity redesign and deleting
`prek_cache_key.py`. The v12 key returns to the existing compatibility
assumptions; the PR description now explicitly avoids claiming broader
cross-runner safety.
##########
.github/actions/install-prek/action.yml:
##########
@@ -46,18 +46,17 @@ runs:
- name: "Compute prek cache key"
id: cache-key
shell: bash
- # Built here rather than assembled from a version each caller formats
for itself, which is how
- # the readers ended up asking for keys no job had ever saved under. uv
resolves the hook
- # environments against the Python on PATH, so callers must run this
after Breeze sets it up.
+ # Include the actual host ABI/OS and absolute environment paths, not
only the target
+ # image platform. Restoring an archive is still followed by prek's
environment validation.
env:
PLATFORM: ${{ inputs.platform }}
UV_VERSION: ${{ steps.versions.outputs.uv-version }}
- PREK_CONFIG_HASH: ${{ hashFiles('**/.pre-commit-config.yaml') }}
- run: |
- PYTHON_VERSION=$(python3 -c 'import platform;
print(platform.python_version())')
-
KEY="cache-prek-v9-${PLATFORM}-python${PYTHON_VERSION}-uv${UV_VERSION}-${PREK_CONFIG_HASH}"
- echo "Prek cache key: ${KEY}"
- echo "key=${KEY}" >> "${GITHUB_OUTPUT}"
+ PREK_VERSION: ${{ steps.versions.outputs.prek-version }}
+ # Dependency changes in local hooks must not reuse an older
environment identity.
+ PREK_CONFIG_HASH: >-
+ ${{ hashFiles('**/.pre-commit-config.yaml',
'**/.pre-commit-hooks.yaml',
Review Comment:
Addressed in 94c15a9bc8: reverted to only
`hashFiles('**/.pre-commit-config.yaml')`. The key is now a readable v12 key
containing only platform, host Python, uv, prek, and the config hash.
##########
scripts/ci/prek_cache_key.py:
##########
@@ -0,0 +1,71 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Compute a cache identity for prek's host-side hook environments."""
+
+from __future__ import annotations
+
+import hashlib
+import json
+import os
+import platform
+import sys
+import sysconfig
+from collections.abc import Mapping
+from pathlib import Path
+
+REQUIRED_INPUTS = ("PLATFORM", "UV_VERSION", "PREK_VERSION",
"PREK_CONFIG_HASH", "GITHUB_WORKSPACE")
+
+
+def build_cache_identity(environ: Mapping[str, str]) -> dict[str, str]:
+ """Do not reuse virtualenvs across incompatible hosts or absolute
interpreter paths."""
Review Comment:
Obsolete after 94c15a9bc8: `prek_cache_key.py` and these docstrings were
removed with the broad identity redesign.
##########
scripts/ci/prek_cache_key.py:
##########
@@ -0,0 +1,71 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Compute a cache identity for prek's host-side hook environments."""
+
+from __future__ import annotations
+
+import hashlib
+import json
+import os
+import platform
+import sys
+import sysconfig
+from collections.abc import Mapping
+from pathlib import Path
+
+REQUIRED_INPUTS = ("PLATFORM", "UV_VERSION", "PREK_VERSION",
"PREK_CONFIG_HASH", "GITHUB_WORKSPACE")
Review Comment:
Obsolete after 94c15a9bc8: the identity helper and its `GITHUB_WORKSPACE`
requirement were removed.
##########
.github/actions/install-prek/action.yml:
##########
@@ -46,18 +46,17 @@ runs:
- name: "Compute prek cache key"
id: cache-key
shell: bash
- # Built here rather than assembled from a version each caller formats
for itself, which is how
- # the readers ended up asking for keys no job had ever saved under. uv
resolves the hook
- # environments against the Python on PATH, so callers must run this
after Breeze sets it up.
+ # Include the actual host ABI/OS and absolute environment paths, not
only the target
Review Comment:
Addressed in 94c15a9bc8: restored both parts of the original
comment—centralized construction prevents writer/reader key drift, and callers
must run after Breeze establishes the Python on PATH.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
