Skip to content

build(deps): replace serde_yaml with serde-saphyr - #39815

Merged
antiguru merged 3 commits into
MaterializeInc:mainfrom
antiguru:deps-serde-saphyr
Oct 15, 2026
Merged

antiguru merged 3 commits into
MaterializeInc:mainfrom
antiguru:deps-serde-saphyr

Conversation

@antiguru

Copy link
Copy Markdown
Member

serde_yaml is deprecated, and it depends on the unmaintained unsafe-libyaml (RUSTSEC-2024-0449). mz-debug and mz-metrics-catalog only write YAML, so this PR moves both to serde-saphyr, built with only its serialize feature. kube-client already depends on serde-saphyr since #39355. With serde_yaml gone, unsafe-libyaml leaves the dependency tree, and its advisory ignore in deny.toml goes with it.

The PR also regenerates doc/user/data/metrics.yml. Its content is unchanged: all 799 entries parse to the same data as before. The formatting differs in two ways. Quoted names use double quotes instead of single quotes, and long help strings become folded block scalars. YAML that mz-debug writes for Kubernetes objects changes in the same ways.

It also drops the ignores for RUSTSEC-2026-0173 and RUSTSEC-2026-0253, which cargo deny no longer detects on main.

🤖 Generated with Claude Code

`serde_yaml` is deprecated and depends on the unmaintained `unsafe-libyaml`
(RUSTSEC-2024-0449). `mz-debug` and `mz-metrics-catalog` only serialize
YAML, so both move to `serde-saphyr` with only the `serialize` feature.
`kube-client` already depends on `serde-saphyr`. With `serde_yaml` gone,
`unsafe-libyaml` leaves the dependency tree and its advisory ignore goes.

The regenerated `doc/user/data/metrics.yml` has identical content but
different formatting: double quotes instead of single quotes, and folded
block scalars for long help strings.

Also drop the ignores for RUSTSEC-2026-0173 and RUSTSEC-2026-0253, which
`cargo deny` no longer detects.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@antiguru
antiguru requested a review from a team as a code owner October 14, 2026 11:50
@antiguru
antiguru requested a review from Alphadelta14 October 14, 2026 11:50

@Alphadelta14 Alphadelta14 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

pretty straightforward. I just had run through this but had to adopt the diverged Error interfaces, which doesn't seem to apply here.

@antiguru
antiguru enabled auto-merge (squash) October 14, 2026 11:56
@def-

def- commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

QA LLM Review

1. MEDIUM -- Catalog name regexes keep the new double quotes, so catalog-diff reports 12 live metric families as removed

.agents/skills/mz-release-signoff/scripts/catalog_names.py:20

catalog_names.py and lint_metrics.py extract metric names from doc/user/data/metrics.yml with ^- name: '?(.+?)'?$, which strips only single quotes. serde-saphyr writes the 12 glob names as "*_requests_total" etc., so both scripts now return names that still carry literal " characters. As a result, catalog-diff.sh misreports live metrics during release sign-off, and lint-skill-metrics.sh fails.

Details

Running catalog-diff.sh ae4f9b74f7 3582f70324 lists all of *_postgres_connpool_*, *_request_duration_seconds_*, *_requests_active and *_requests_total under "Removed (an after-window on these is empty by construction)", with quoted copies under "Added". The release sign-off skill says to treat a name in the removed list as "a documentation fix rather than a finding". On any sign-off whose tag range spans this PR, an empty HTTP-request or persist-connpool panel would therefore be explained away when it could be a real regression. CI does not exercise catalog_names.py.

lint_metrics.py:24 has the same regex. That is why lint-docs is red on this PR: mz_http_requests_total and mz_http_request_duration_seconds_bucket no longer match the quoted globs. The lint message suggests adding them to the allowlist, but that would hide the real cause.

Fix: accept both quote styles in both scripts, for example ^- name: (['"]?)(.+?)\1$ (group 2). Old and new tags then parse to the same names.

@antiguru
antiguru disabled auto-merge October 14, 2026 12:08
serde-saphyr quotes glob metric names with double quotes, and the catalog
name regexes only stripped single quotes. Both quote styles now parse to the
same names, so catalog diffs across old and new refs stay correct.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@antiguru

Copy link
Copy Markdown
Member Author

Fixed in 21eea58. Both catalog_names.py and lint_metrics.py now accept either quote style via ^- name: (['"]?)(.+?)\1$. catalog_names.py returns the same name set for ae4f9b74f7 (single quotes) and this branch (double quotes), and ci/test/lint-skill-metrics.sh passes locally. The Hugo metrics-table shortcode reads the file through a YAML parser, so the quote change doesn't affect it.

Posted by Claude Code

The name extractors matched catalog lines with a regex, which broke when the
generator changed its quoting style. Parse the catalog with PyYAML instead,
so formatting changes in `bin/gen-metrics-catalog` output cannot change the
extracted names. Both scripts now run through `bin/pyactivate`, which
provides PyYAML.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@antiguru
antiguru requested a review from a team as a code owner October 14, 2026 13:48
@antiguru

Copy link
Copy Markdown
Member Author

3bbb1e1 replaces the regex fix: catalog_names.py and lint_metrics.py now parse the catalog with yaml.safe_load and run through bin/pyactivate for PyYAML. Changes to the generator's formatting can no longer change the extracted names. catalog-diff.sh ae4f9b74f7 HEAD reports no added or removed metrics, and ci/test/lint-skill-metrics.sh passes.

Posted by Claude Code

@antiguru
antiguru requested a review from Alphadelta14 October 14, 2026 13:51
@antiguru
antiguru merged commit 0d8ae7c into MaterializeInc:main Oct 15, 2026
84 checks passed
@antiguru
antiguru deleted the deps-serde-saphyr branch October 15, 2026 07:41
@antiguru

Copy link
Copy Markdown
Member Author

Thanks for the review!

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.

3 participants