Skip to content

4.4.3-4: SZ_CHECK_LICENSE, --license-file, resize reaches running resolvers - #11

Merged
brianmacy merged 1 commit into
mainfrom
sync-4.4.3-4
Nov 2, 2026
Merged

brianmacy merged 1 commit into
mainfrom
sync-4.4.3-4

Conversation

@brianmacy

Copy link
Copy Markdown
Contributor

Public docs and changelog for 4.4.3-4.

@brianmacy
brianmacy requested a review from a team as a code owner November 2, 2026 16:24
@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown

🤖 Claude Code Review

PR Code Review: 4.4.3-4

This diff changes only documentation (CHANGELOG.md, docs/install.md, docs/troubleshooting.md, docs/use.md). I did not run prettier. Line numbers below come from the diff hunks, so they are approximate.

Code Quality

  • ✅ Style, naming, commented-out code, DRY: nothing to flag, and there is no code in the diff.

  • ❌ Defects: the code behind the documented features is not in this diff. The docs and CHANGELOG describe these changes:

    • API.SZ_CHECK_LICENSE
    • the --license-file install flag
    • SZ_CONFIGURE and SZ_SET_CONFIG applying resolver limits to running services

    I could not review any of that behavior. If the code is in this PR, it was left out of the diff I was given. If it was merged earlier, the CHANGELOG should say so.

  • ⚠️ CHANGELOG.md ~L13–43: version split. The old [Unreleased] content (resolver platform metrics and so on) is now filed under a new ## [4.4.3-3] - 2026-11-02 heading. Check that this is intended. Two release headings share one date, and the CHANGELOG header says the -N suffix restarts at 1 for each Senzing version, which is consistent. Also check that the link references at the bottom of the file, if the file uses them, were updated for 4.4.3-3 and 4.4.3-4.

  • ✅ Project memory (.claude/CLAUDE.md): not touched by this diff.

  • ✅ Consistency of the resolver sizing numbers in docs/use.md (~L226–235): the new STANDARD_XL example is correct. 4 cores at 2 resolvers per core gives 8 resolvers. 8 × 9 + 13 = 85 sessions, which is under the 160 limit.

  • ✅ CHANGELOG.md and docs/use.md agree that the earlier CPU-autoscaling claim was wrong.

Testing

  • ❌ Unit and integration tests: no tests are in the diff for SZ_CHECK_LICENSE:
    • the -20111 validation
    • the "valid": false path
    • nothing being stored or restarted
    • the --license-file pre-checks (empty, non-base64, length not a multiple of 4)
    • applying config changes to running resolvers
  • ❌ Edge cases: the following look untested here:
    • whitespace or newlines in a license file
    • a trailing newline from base64 output
    • URL-safe base64
    • a license containing a single quote, which would break the ALTER SECRET ... SET SECRET_STRING = '...' statement the script sends
  • ❌ Coverage > 80%: I can't assess it from the diff.

Documentation

  • ✅ README, API docs and inline comments: install.md, troubleshooting.md and use.md are updated and cross-consistent (the -20111 meaning is the same in all of them).
  • ✅ CHANGELOG updated.
  • ⚠️ Markdown formatting:
    • The new CHANGELOG lines are hard-wrapped at ~95 columns, while the neighboring existing entries are one long line. Pick one style, or whichever prettier's proseWrap setting enforces.
    • docs/install.md lines are wrapped inconsistently, and the paragraph beginning "ALTER SECRET ... or give the install script" now has uneven line lengths. Run prettier over the changed files.
  • ⚠️ docs/install.md renewal example: the SQL comment -- {"valid": true, ...}: then store it and reload is ambiguous because it mixes output and instruction. Consider putting the expected result in a separate comment line.

Security

  • ✅ No hardcoded credentials or checked-in license files. <base64> is a placeholder, and nothing in the diff starts with AQAAAD.

  • ⚠️ Sensitive data exposure (documented, but worth noting). The license is visible in two places:

    • the check job's spec while the job runs, readable by anyone with job-inspection privileges on the account
    • query history, from both the CALL and the ALTER SECRET

    The docs disclose both. If possible, the check job should read the license from a secret rather than the job spec, so it is not visible there.

  • ❌ Input validation and error handling: the validation described (empty, base64, multiple of 4) is a good first line. I can't confirm it is implemented, or that single quotes are escaped when the script builds the ALTER SECRET statement. This ties back to the missing code.

Summary

The docs and CHANGELOG are clear and internally consistent, and the sizing arithmetic checks out. The blockers are that the implementation and tests for SZ_CHECK_LICENSE, --license-file and the running-resolver resize are not in this diff, and that I could not run prettier. Please also verify the CHANGELOG version split and the link references.

Automated code review analyzing defects and coding standards

@brianmacy
brianmacy enabled auto-merge (squash) November 2, 2026 16:28
@brianmacy brianmacy self-assigned this Nov 2, 2026
@brianmacy
brianmacy disabled auto-merge November 2, 2026 16:30
@brianmacy
brianmacy merged commit 5240527 into main Nov 2, 2026
8 checks passed
@brianmacy
brianmacy deleted the sync-4.4.3-4 branch November 2, 2026 16:30
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.

1 participant