Add changelog generation tool for GitHub milestones - #13063
Conversation
|
hmm, the uv.lock file is making the RAT check mad. Should I remove uv.lock? |
I think it's recommended to add uv.lock to ensure the exact packages are added. Let's add it back. The problem wasn't your patch adding the lock, the problem is the RAT check incorrectly failing on the lock file. I'll update CI to allow it. Update |
2b949cf to
44f4931
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces a new Python-based changelog generator under tools/changelog/ to produce CHANGELOG-*-style output from GitHub milestones (via direct REST API calls or the gh CLI), and updates the release-process documentation to use it.
Changes:
- Add
tools/changelog/changelog.pywith text/YAML output and an extended--docmode for richer metadata. - Add
tools/changelog/pyproject.tomlandtools/changelog/uv.lockfor dependency management/execution viauv. - Update release-process docs to use the new tool (with
--use-gh).
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| tools/changelog/changelog.py | Implements milestone PR collection via REST API or gh, plus output formatting. |
| tools/changelog/pyproject.toml | Defines the Python project and console script entry point. |
| tools/changelog/uv.lock | Pins Python dependencies for uv-managed execution. |
| doc/developer-guide/release-process/index.en.rst | Updates the documented release workflow to generate changelogs via the new tool. |
Replaces tools/git/changelog.pl with a Python implementation that generates changelogs from merged PRs in a milestone using the GitHub API or gh CLI. Default output matches the existing CHANGELOG-* file format. The --doc mode includes merge SHAs, labels, and full PR descriptions to guide AI-assisted release documentation updates. Supports text and YAML output formats. Co-Authored-By: Claude <noreply@anthropic.com>
Replace reference to tools/git/changelog.pl with the new tools/changelog/changelog.py invocation using uv run. Co-Authored-By: Claude <noreply@anthropic.com>
e208668 to
cd07fa5
Compare
There was a problem hiding this comment.
Pull request overview
Replaces the legacy Perl-based milestone changelog generator with a new Python tool under tools/changelog/ that can pull merged PRs for a milestone via the GitHub REST API or the gh CLI, and updates the release process docs accordingly.
Changes:
- Removed
tools/git/changelog.pl(Perl implementation). - Added a Python-based changelog generator (
tools/changelog/changelog.py) with apyproject.toml+uv.lockfor dependency management. - Updated release-process documentation to use the new tool.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/git/changelog.pl | Removes the old Perl changelog generator. |
| tools/changelog/changelog.py | New Python tool for generating milestone changelogs via GitHub API or gh, with optional doc/YAML output. |
| tools/changelog/pyproject.toml | Defines the Python tool project and dependencies. |
| tools/changelog/uv.lock | Locks Python dependencies for reproducible runs via uv. |
| doc/developer-guide/release-process/index.en.rst | Updates release instructions to use the new Python tool. |
| parser.add_argument("-m", "--milestone", required=True, help="Milestone title") | ||
| parser.add_argument("-a", "--auth", default=None, help="GitHub auth token (or set GH_TOKEN env var)") | ||
| parser.add_argument("-v", "--verbose", action="store_true", help="Verbose output") |
|
[approve ci autest 2] |
bryancall
left a comment
There was a problem hiding this comment.
Nice to see this move off Perl. The default httpx path looks solid: explicit rate-limit checks, raise_for_status, distinct exit codes, and a good unauthenticated-token warning.
One thing I want fixed before merge. In the --use-gh path, the per-PR merge check treats any non-zero gh api .../merge exit as "not merged" and skips the PR (changelog.py L130). That collapses a genuine 404 (really not merged) together with 403 secondary rate limiting, 5xx, and network errors into the same outcome. Since this makes one API call per PR across a whole milestone, secondary rate limiting is exactly the failure to expect, and when it hits you get a silently incomplete release changelog with a zero exit code. The httpx path already does this right in _is_merged (204 vs 404 vs raise_for_status). Please have the gh path distinguish 404 from other failures and error out on the rest instead of silently skipping. Same goes for the --doc detail fetch, which substitutes empty sha/body on failure rather than surfacing it.
Smaller items, not blocking:
--dochelp and the module docstring say "full commit message" but the code stores the PR body. Fix the wording (or fetch the actual commit message).- The
-a/--authtoken is visible inpsand shell history. Carried over from the old script, but for new code prefer GH_TOKEN only and mark-aas discouraged. pyproject.tomlis missinglicense = "Apache-2.0"that the other tool packages set, andmain()is missing a-> Nonereturn annotation.
The Copilot notes about milestone state=all and a --format yaml JSON fallback are already handled in the current code: both milestone lookups use state=all, and yaml exits with a clear error when PyYAML is missing.
* Clarify HostDBInfo state * Address comments from Copilot * Cleanup comments
This fixes a crash where the assertion at HttpSM.cc:2765 fails: ink_assert(default_handler != (HttpSMHandler) nullptr) The crash is triggered when VC_EVENT_EOS, VC_EVENT_ERROR, or a timeout propagates from the QUIC stream up to HQTransaction::state_stream_open, which calls _signal_event. The previous implementation forwarded the triggering Event* as the data argument to HttpSM::handleEvent. HttpSM::main_handler, however, casts that data to VIO* and uses it to look up the vc_table entry. Because an Event* never matches any registered VIO, find_entry returns null and main_handler falls through to default_handler. When the SM is in early setup or has already been torn down (kill_this clears default_handler), default_handler is null and the assertion fires. This patch passes the appropriate VIO pointer to each handleEvent call, matching the convention used by _signal_read_event, _signal_write_event, and the HTTP/2 Http2Stream equivalents. The original dual-VIO dispatch is preserved so tunnel consumers bound to the write side still receive connection-level events. Fixes: apache#12112
New settings: proxy.config.ssl.server.cert_compression.algorithms proxy.config.ssl.client.cert_compression.algorithms proxy.config.ssl.server.cert_compression.cache is going to be added on next PR. New metrics: proxy.process.ssl.cert_compress.<alg> proxy.process.ssl.cert_compress.<alg>_failure proxy.process.ssl.cert_decompress.<alg> proxy.process.ssl.cert_decompress.<alg>_failure
…3115) * Move unit tests under unit_tests dir * Cleanup HostDB unit tests and benchmark
* Cap uncompressed length in TLS Certificate Compression * Set a nullptr * Update the comment
Replace the single global geo DB handle with a cache of handles keyed by path, allowing different remap rules to use different MMDB files via --geo-db-path. The handle is threaded through RulesConfig -> Resources -> ConditionGeo::get_geo_*() methods. The cache deduplicates when multiple rules share the same path. (cherry picked from commit 8713ad4) Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Increase autest coverage with the following tests: - TLS renegotiation, rejected by default and allowed when configured - the TLS record-size clamp, checked against on-the-wire record sizes - a failed outbound TLS origin connection surfaced as a 5xx - a TLS origin that resets the connection mid-request-body
* Annotate BRAVO locks for thread-safety analysis Enabling -Wthread-safety tree-wide surfaced findings in Bravo.h on macOS, where libc++ marks the underlying std::shared_mutex as a capability: the wrapper acquires and releases it across method boundaries and on fast/slow-path branches the analyzer cannot follow. Fedora CI uses libstdc++, which does not annotate std types, so it never saw these. Make ts::bravo::shared_mutex_impl a real capability so the contract is checked on every Clang, not papered over on one std lib; its lock-driving bodies are the trusted implementation and stay exempted, matching the ts::shared_mutex pattern. Make the reader guard a rigid scoped capability -- acquire in constructor, release in destructor, no copy/move/defer/release -- which the analysis tracks with no exemption; the movable std::shared_lock-style forms it replaced could not be tracked, and no caller used them. Restructure the unit test's try-lock asserts so the acquire gates a branch. * Reset BRAVO Token on entry to harden slow-path unlock lock_shared/try_lock_shared assigned the Token only on the fast path, so a reused non-zero Token surviving into a slow-path acquisition would make unlock_shared release a reader slot instead of the underlying mutex. Clear it on entry to enforce the documented 0-init contract.
* Self-describing binary log format (LogBuffer v3)
Publish each field's type in a per-segment schema so a generic reader can
decode a .blog from the file alone, without an embedded ATS symbol-to-type
table that must track the writer in lockstep. The per-field code is
LogField::Type serialized directly (now an enum class : uint8_t with INVALID=0
reserved and sINT..IP = 1..4 as the frozen wire codes); a static_assert pins
the values. This relies on each field's declared type matching its marshalled
framing, which the parent commit ("Fix mismatched sINT/dINT log field types")
establishes.
Readers (LogBufferIterator, logcat, logstats, the ASCII output paths) accept
both v2 and v3 segments, sizing the header read to the on-disk version, so a v3
build keeps decoding logs written by an older one. Integer values stay in host
byte order, as in v2 (no endianness change). The public TSLogType enum is given
the same values as LogField::Type so TSLogFieldRegister can static_cast between
them; static_asserts in InkAPI.cc (the only TU that sees both) pin the
alignment so a future reorder fails to compile.
The writer version is per-LogObject: logging.yaml "binary_log_version: 2"
pins a binary log to the pre-v3 layout (no schema, shorter header) so a
not-yet-upgraded downstream parser keeps working during a migration; the
default is v3.
Decoding untrusted .blog input is bounded: LogBufferIterator validates
data_offset and each entry against the segment, and the JSON decoder validates
the schema offset alignment and cross-checks field_count against the symbol
list.
* Address Copilot's comment
* Address Copilot's comment
* Cleanup
* Fix logcat for AuTest
* Range-check untrusted .blog offsets before pointer arithmetic
fmt_fieldlist() and fmt_fieldtypes() form a pointer from a header
offset read off disk; an out-of-range value makes the pointer
arithmetic undefined behavior even if never dereferenced. Guard
both against byte_count.
Before this patch, the client ip debug logging test only covered one HTTP transaction, so regressions across protocols or persistent client sessions could pass unnoticed. This converts the test to replay-driven coverage for HTTP, HTTPS, and HTTP/2, with an HTTP/3 scenario enabled when QUICHE is available. Each replay sends multiple transactions on one client connection and checks that all four request and response header dumps include per-transaction markers. This test found no issues, thus this is a test-only patch.
JSONRPC clients use nonblocking Unix sockets, so a full peer receive buffer can make write return EAGAIN. The old retry loop treated that like EINTR and immediately retried forever, which could hang the jsonrpc server tests during large request and restart coverage. This waits for socket write readiness before retrying transient backpressure and reports ETIMEDOUT when the readiness wait expires, while preserving readiness poll errors. This also adds a regression test that fills a socketpair and verifies that the helper returns with that timeout signal instead of spinning.
…apache#13312) ssl_callback_ocsp_stapling() emitted an Error on every TLS handshake when a cert's OCSP response was missing or expired, which could flood error.log. Use SiteThrottledError so ops are still alerted at Error severity but the message is rate-limited per call site (default 60s) with a suppressed-count. Co-authored-by: Evan Zelkowitz <e_zelkowitz@apple.com>
…ty (apache#13360) ServerBacktrace() reports success but yields no frames when the target's thread list is unreadable -- e.g. a fast-aborting target has already exited by the time the forked helper attaches. The success check only tested for a null trace, so an empty-but-non-null trace fell into the success path and produced a report with no backtrace and no explanation. Require a non-empty trace before treating ServerBacktrace as having succeeded, so the empty case falls through to the existing in-process-backtrace and diagnostic-message fallback.
* docs: add call condition note for TSUrlHostGet Add a note to the TSUrlHostGet documentation indicating that it should only be called after TS_HTTP_POST_REMAP_HOOK. For earlier hooks like TS_HTTP_READ_REQUEST_HDR_HOOK, TSHttpHdrHostGet should be used instead. Fixes apache#5742 * docs: fix unknown interpreted text role data Replace :data: with double backticks for hook names, consistent with the rest of the documentation. Fixes apache#5742 * docs: trigger CI for TSUrlHostGet call condition note * docs: document TSHttpHdrUrlGet hook availability, fix redirect_1 to use TSHttpHdrHostGet - TSHttpHdrUrlGet: add note that URL components may not be available at early hooks, recommend TSHttpHdrHostGet for reliable host retrieval - TSUrlHostGet: add call condition note (TS_HTTP_POST_REMAP_HOOK onwards) and cross-reference to TSHttpHdrHostGet - TSHttpHdrHostGet: add cross-references to TSHttpHdrUrlGet and TSUrlHostGet - redirect_1: replace TSHttpHdrUrlGet+TSUrlHostGet with TSHttpHdrHostGet which works correctly at TS_HTTP_READ_REQUEST_HDR_HOOK * fix(docs): replace :c:macro: with double backticks for TS_HTTP_READ_REQUEST_HDR_HOOK * fix(docs): replace :c:macro: with double backticks for TS_HTTP_READ_REQUEST_HDR_HOOK * fix(docs): replace :c:macro: with double backticks for TS_HTTP_READ_REQUEST_HDR_HOOK * fix: restore TSHttpHdrHostGet docs content and add cross-references - Restored accidentally emptied TSHttpHdrHostGet.en.rst - Added call condition note explaining TSHttpHdrHostGet vs TSUrlHostGet - Added See Also cross-references to related APIs Fixes docs build warning reported by JosiahWI * fix: add trailing newline to redirect_1.cc * fix: restore upstream license header formatting * fix: restore upstream license header formatting * fix: restore upstream license header formatting * fix: restore upstream license header in redirect_1.cc * fix: restore license header and fix broken string literal * fix: restore blank line after title underline * fix: add blank line between title and Synopsis * fix: apply code changes on top of upstream cleanly * fix: restore redirect_1.cc with correct API changes * fix: restore redirect_1.cc with correct TSHttpHdrHostGet usage * fix: apply TSHttpHdrHostGet changes cleanly on upstream * fix: restore blank line between title and Synopsis --------- Co-authored-by: Mustafa Senoglu <mustafa@senoglu.local>
…ock (apache#13343) A selector configured without a `metrics:` block should be a metrics no-op. It isn't: _metrics is value-initialized to 0, but incrementMetric() only suppresses the update when an entry equals the TS_ERROR (-1) sentinel that metric_helper() uses for "not registered". So the guard never fires and every queue/reject/expire/resume calls TSStatIntIncrement() on an unregistered ID. (It currently lands on the reserved bad_id slot, so it is absorbed rather than fatal -- but the plugin still should not emit anything.) Default _metrics to TS_ERROR in the constructor so incrementMetric() stays a no-op until metrics are actually registered.
Add USDT probes to the following: - iocore/net: socket read/write, read/write disable, reenable, do_io_close, inactivity timeout, TLS read - iocore/cache: read-while-writer attach, produce, starve, writer close - proxy/http: tunnel producer/consumer/flow-control, add consumer, chunk decode, UA abort, transfer setup, background fill - proxy/http2: send-window/write-buffer block, data frame, window update, RST_STREAM sent/received The existing probes only mark one-shot lifecycle milestones; these cover the steady-state body-transfer and flow-control phases. They compile to no-ops unless built with -DENABLE_PROBES=ON.
The cqssrt (client_req_ssl_resumption_type) log field added in apache#12404 was silently dropped from master by the 11-Dev integration merge (apache#12983, 8415cef). A criss-cross merge resolution removed its registration in Log.cc and the LogAccess marshal path, while the supporting HttpUserAgent::get_client_ssl_resumption_type() machinery survived, leaving that accessor as dead code with no caller. Re-register the field and re-add the marshal function plus the TransactionLogData bridge accessor so the orphaned machinery is wired back into the log system. Declare it as sINT (it marshals a single int), not the original dINT, matching the type/marshal-framing fix in apache#13223.
…pache#13371) BoringSSL rejects a peer-initiated renegotiation inside the library before ATS's SSL info callback runs -- SSL_get_state() there only ever returns SSL_ST_INIT or SSL_ST_OK, never SSL_ST_RENEGOTIATE -- so the "trying to renegotiate from the client" line is never logged and the ContainsExpression fails on BoringSSL (Apache CI stays green because it runs OpenSSL). The crash-safety check still runs on every SSL library; only the detection-line assertion is now OpenSSL-only.
…pache#13338) * jax_fingerprint: Reduce allocations and gate methods at build time Trim per-connection memory work in the hybrid (global + remap) setup by collapsing the per-connection table of fingerprint contexts to an inline structure and by passing fingerprints into the context without an intermediate copy. Lookup behavior is unchanged. Add ENABLE_JAX_METHODS as the configure-time switch for which fingerprint methods are compiled in. CMake derives the per-method preprocessor defines, the dispatcher table in plugin.cc, and the slot count of the inline context table from the same list. An empty list or an unknown method directory fails at configure time. Include a developer README covering the per-method file layout, the build-time switches, and the naming rules that the CMake glob relies on. * Address copilot comments
…13333) The head/range/redirect auth transforms copy the whole client request and then force it bodyless (method override + Content-Length: 0) to probe the auth server, but left Transfer-Encoding, Trailer, and Expect in place. A chunked or Expect: 100-continue client request therefore produced a self-contradictory sub-request: a bodyless HEAD/GET still advertising a body. ATS honors the framing and sets up a request-body tunnel for a body that never arrives, stalling the probe until the inactivity timeout; and proxy.config.http.reject_head_with_content rejects a HEAD that declares content outright. Strip Transfer-Encoding, Trailer, and Expect when normalizing the sub-request to bodyless.
The async_handshake test plugin is only built with OpenSSL (TS_USE_TLS_ASYNC). SkipUnless does not evaluate its conditions where it appears; it only registers them for the framework to check later, so the test file keeps executing and PrepareTestPlugin ran at load time and raised a ValueError when the plugin was missing, reported as a test exception instead of a skip. Guard the call on file existence so the test skips cleanly on non-OpenSSL builds.
* docs: document the wipe_field_value logging filter Add a dedicated section describing how the wipe_field_value log filter masks query parameter values, including that it matches parameter names rather than values. The worked example is taken from the existing log-filter autest so the shown output matches what Traffic Server actually produces. * Address copilot comments
The stale_response log checks can run before every directive that they later assert has been written. Waiting for one marker with a sleep-based process leaves the final content checks exposed to ATS log flush timing when both stale directives are expected. This replaces the sleep-based watcher with explicit await runs for each directive being asserted. The test now waits for the matching stale-while-revalidate and stale-if-error entries before performing the final log content checks. Fixes: apache#13301
Default server certificate secret updates could rebuild the TLS contexts for CN/SAN lookups while leaving the default/no-SNI context pointing at the old SSL_CTX. Operators could update cert material on disk and through the secret API, but new handshakes without a more specific match could still serve the stale certificate. This updates runtime context refresh to cover address/default lookup entries owned by the same ssl_multicert policy and retains the default context while callers create new TLS sessions. This also adds an AuTest that updates a plugin-loaded default certificate and verifies the next no-SNI handshake sees the new certificate. Fixes: apache#9562
…3233) PR apache#11733 rewrote the CACHE_VALUE_HITS_SIZE cast so static_cast<float> wraps the whole quotient, making (hits + 1) / (size + overhead) integer division. It truncates to 0 for normal object sizes, zeroing the value metric and collapsing CLFUS to FIFO: no promote-on-hit, no clock second chance, and no value-based ghost re-admission. Bind the cast to the numerator to restore floating-point division, and add the ram_cache_clfus_value regression test as a guard (it fails on the pre-fix macro and passes after).
HostStatus lookups sit on the parent-selection hot path and are overwhelmingly reads, so replace the ink_rwlock with the reader-biased BRAVO shared_mutex for better read scaling under contention. Readers take ts::bravo::shared_lock (Token-aware); writers use std::scoped_lock.
fmt 11 dropped fmt::format/vformat from fmt/core.h; include fmt/format.h instead, and use fmt::format directly to avoid a deprecated format_string conversion. Still builds on fmt 8.1+.
* Reject over-long unix socket paths in server_ports A path longer than sun_path (108 bytes including the terminator) was silently truncated, and left unterminated, since strncpy writes no terminator when it truncates, so ATS bound a listener on the wrong filesystem path with nothing telling the operator the configured path was too long. Reject the path at configuration parse with a Warning naming the limit, and make UnAddr's string constructors always null-terminate.
Replaces tools/git/changelog.pl with a Python implementation that generates changelogs from merged PRs in a milestone using the GitHub API or gh CLI. Default output matches the existing CHANGELOG-* file format. The --doc mode includes merge SHAs, labels, and full PR descriptions to guide AI-assisted release documentation updates. Supports text and YAML output formats.