-
Notifications
You must be signed in to change notification settings - Fork 16.4k
feat: add a script to verify NOTICE files and update release documentation to include its use. #60674
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
base: main
Are you sure you want to change the base?
Conversation
…ation to include its use.
dev/README_RELEASE_PYTHON_CLIENT.md
Outdated
| For distribution packages, after building: | ||
|
|
||
| ```shell script | ||
| python3 dev/verify_notice_files.py --dist-only --year 2026 --verbose |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would the year not be auto-discoverable by the script? Maybe to be able to optionally override the year if release is cut on Dec 31st and somebody validates on Jan 1st.... but in all other cases we are in the same year.
If you have verbose by default, is logging then set correct? Or does a verbose output need to be inspected for each release check? I'd favor less verbosity in default and a clear PASS/ERROR result.
|
It feels that there is a lot of AI -generated content that could be simpliified a lot if you assume that you have a prek hook that check individual files (which we will have to do anyway), I just (also using Claude Sonnet) generated minimal prek hook doing such verification #60699 and it's ** really simple **. Take a look as an example. Again I asked Claude to generate minimal verification script without and configuration parameters - because we generally do not need them And it proposed this: bash: set -euo pipefail
tmpdir=$(mktemp -d)
trap "rm -rf $tmpdir" EXIT
for file in dist/*.{whl,tar.gz}; do
[[ -f "$file" ]] || continue
unzip -q -j "$file" "*/NOTICE" -d "$tmpdir" 2>/dev/null || \
tar -xzf "$file" --wildcards "*/NOTICE" -C "$tmpdir" --strip-components=1 2>/dev/null || true
done
notices=("$tmpdir"/NOTICE*)
[[ -f "${notices[0]}" ]] && ./scripts/ci/prek/check_notice_files.py "${notices[@]}"
Python: from __future__ import annotations
import glob
import subprocess
import tarfile
import tempfile
import zipfile
from pathlib import Path
with tempfile.TemporaryDirectory() as d:
for f in glob.glob("dist/*.{whl,tar.gz}", recursive=True):
try:
zipfile.ZipFile(f).extract(
next(n for n in zipfile.ZipFile(f).namelist() if n.endswith("NOTICE")), d
)
except:
try:
tarfile.open(f).extract(
next(n for n in tarfile.open(f).getnames() if n.endswith("NOTICE")), d
)
except:
pass
if n := list(Path(d).rglob("NOTICE")):
subprocess.run(["./scripts/ci/prek/check_notice_files.py", *map(str, n)])It probably needs a b it polish. but it's generally way, way smaller and does the same job IMHO. |
…nt parsing and complex logic, and update its usage in release documentation.
|
Thanks for the feedback! I've simplified the script:
The script finds all source NOTICE files + extracts from dist packages. |
Description
Added:
verify_notice_files.py: Script to validate NOTICE files in both source tree and distribution packages (.tar.gz, .whl)Verification checks:
Integration: Added verification steps to all 5
README_RELEASE_*.mdfilesFlags: Supports
--sources-onlyand--dist-onlyfor flexible verification during development and releaseTesting
Future Work
As mentioned in #60540:
If this approach is approved, I'd be happy to work on these enhancements in a follow-up PR.
Closes #60540
Was generative AI tooling used to co-author this PR?
Generated-by: Claude Sonnet 4.5, following the guidelines
{pr_number}.significant.rstor{issue_number}.significant.rst, in airflow-core/newsfragments.