feat(python): add snippet tests and Linux CI by antaenc · Pull Request #248 · Senzing/code-snippets-v4

antaenc · 2026-10-09T19:42:03Z

Part of #11 (testing and workflows; no Makefile yet).

What

python/tests: pytest runs every Python snippet end to end as a subprocess, each against its own copy of a temporary SQLite repository. Snippets are discovered automatically; the SNIPPETS table in test_snippets.py covers the ones needing data loaded, data sources registered, console input or a ctrl-c (the Python equivalent of the Java runner's .properties files). A test passes on a clean exit and empty stderr, because per-record errors are logged to stderr without changing the exit code.
Workflow: python-linux-snippets.yaml, modelled on the Java Linux snippet workflow: Python 3.10–3.14 × the SDK-versions matrix. Linux only, because the Python SDK isn't shipped for macOS or Windows.
Snippets now exit 1 (and report to stderr) when a Senzing error stops them. Previously they printed the error and exited 0, so a harness couldn't tell success from failure.
Explicit thread count: the futures and queue snippets use a MAX_WORKERS = 8 constant (matching Java's THREAD_COUNT) instead of reading the executor's private _max_workers. Behaviour change: the default was min(32, cpu_count + 4).

Fixes found along the way

add_queue.py loaded 0 records but exited 0 when the multiprocessing start method isn't fork (the Linux default from Python 3.14 is forkserver). It now uses a producer thread like the Java and C# LoadViaQueue, which also fixes a racy queue.empty() stop condition. Errors in the producer thread (for example a decode error) are handed to the main thread, so the snippet exits 1 instead of reporting a partial load as success.
redo_with_info_continuous.py raised a TypeError instead of exiting on a Senzing error.
resources/output/ wasn't in the repo, so the with-info snippets failed with FileNotFoundError on a fresh clone. It's now tracked via .gitkeep, with its contents still ignored.
redo_continuous_futures.py spun calling get_redo_record() when fewer redo records than workers were waiting (existing bug, flagged in review).
The Bandit CI scan didn't use pyproject.toml, so it flagged assert in the tests despite the configured B101 skip. It now passes configfile and pins bandit-action@v1.0.1, because v1 (v1.0.0) installs bandit without TOML support.

Housekeeping

Release 0.0.11: CHANGELOG section [0.0.11] - 2026-10-09, and pyproject.toml version 0.0.11. The version was previously 1.2.8, which was unrelated to the 0.0.x release tags.
.claude/CLAUDE.md documents running the tests, and the flake8/bandit commands now match the repo config.

Testing

Linux CI: all 10 jobs pass (Python 3.10–3.14 × production/staging SDK).
Locally on Linux (arm64) against SDK 4.4.2: 36 passed, 1 skipped (abstract_factory_parameters.py hardcodes its own settings).
Negative checks: a snippet whose records all fail now fails its test; add_queue.py exits 1 (no hang) on a fatal error with a full queue, and exits 1 on a missing input file or invalid UTF-8 input; redo_continuous_futures.py's refill loop was checked with a fake engine (old code spins, new code pauses).
black, isort, flake8, pylint, mypy (also --strict on the tests), bandit and cspell are clean; super-linter v8.7.0 was run locally with the same config as lint-workflows and passes.

Resolves #11

- Add python/tests: runs every snippet end to end as a subprocess against its own temporary SQLite repository, requiring a clean exit and stderr - Add python-{linux,darwin,windows}-snippets workflows - Snippets exit 1 and report to stderr when a Senzing error stops them - add_queue.py uses a producer thread instead of separate processes; it previously loaded nothing under spawn/forkserver yet exited 0 - signal_handler.py no longer uses signal.pause(), unavailable on Windows - Fix TypeError in redo_with_info_continuous.py error handling - Track resources/output/ so with-info snippets work on a fresh clone - Update CHANGELOG for 0.0.11

github-actions · 2026-10-09T19:43:07Z

🤖 Claude Code Review

Code Review

Code Quality

✅ Style: The code is consistent with the surrounding snippets. Every except SzError block now writes to stderr and exits with status 1. sys is imported wherever it is newly used.
✅ No commented-out code.
✅ Variable names: Names are meaningful. The producer parameter is renamed from queue to record_queue, so it no longer shadows the new queue import.
❌ DRY: The Mint staging … token and install Senzing SDK steps are copied between the darwin and windows workflows. The three workflows also repeat the whole sdk-versions, concurrency and Slack-notify scaffolding. This is acceptable for CI, but a reusable workflow would be cleaner.
❌ Defects:
1. Possible hang in python/loading/add_queue.py:32-38 and :91-96. The consumer stops pulling from the queue once shutdown is set, for example after an SzUnrecoverableError.
  - The producer thread then blocks forever in record_queue.put(record, block=True), because the queue holds at most 200 items and is no longer drained.
  - producer_thread.join() therefore never returns, and the snippet hangs instead of exiting. The finally: put(None) can block the same way.
  - Fix: after the consumer returns, drain the queue, or use a stop event and put with a timeout.
  - daemon=True does not help, because join() is called explicitly.
2. Private attribute at add_queue.py:57. executor._max_workers is private API. It was already used before this PR, but it is worth replacing with an explicit worker count.
3. Workflow trigger inconsistency. The Linux workflow runs on push to main and on pull_request to main. The darwin and windows workflows run on every pull_request and never on push. Main therefore gets no macOS or Windows coverage after a merge, apart from the nightly schedule. Align the triggers.
4. Minor: python/tests/build_repo.py:25. with sqlite3.connect(...) commits but does not close the connection. This is harmless on CPython, but an explicit closing() is safer on Windows, where the file is copied immediately afterwards.
5. python/tests/test_snippets.py. test_snippet_table_is_current only catches stale SNIPPETS entries. Snippets that need special setup but have no entry fail with unclear errors, which is acceptable given the README note.
✅ .claude/CLAUDE.md: Not modified. It would still be worth adding the pytest command to it, and it contains nothing environment-specific.

Testing

✅ Unit tests for new functions: The new tests run every snippet end to end, with automatic discovery. There is no direct test of add_queue.py's producer and consumer error paths. That gap is why the hang above is not caught.
✅ Integration tests: Each snippet runs in a subprocess against its own SQLite repository.
❌ Edge cases: There is no case for the consumer shutting down early with a full queue. On Windows, a killed snippet (result.killed) skips the exit-code assertion, so a crash would be masked.
❌ Coverage > 80%: Coverage is not measured for the snippets, because they run as subprocesses. This is not verifiable from the diff.

Documentation

✅ README: python/README.md documents how to run the tests and the environment variables.
✅ Inline comments: The comments explain the non-obvious parts, such as the Windows kill and the UTF-8 setting.
❌ CHANGELOG:
1. The [Unreleased] section is renamed to [0.0.11] - 2026-10-09, but no [0.0.11] link reference definition was added. The file's reference list contains only [CommonMark], [Keep a Changelog] and [Semantic Versioning]. Neither [Unreleased] nor [0.0.10] has a link definition either, so this is a pre-existing pattern.
2. pyproject.toml has version = "1.2.8", which does not match 0.0.11. Releasing in a feature PR is questionable. Consider keeping the entries under [Unreleased].
✅ Markdown: The new text appears prettier-compatible. I did not run prettier.
✅ cspell: The new words were added to .vscode/cspell.json.

Security

✅ Hardcoded credentials: None. The na:na string is the SQLite placeholder in the test settings. Secrets come from secrets.*.
❌ Workflow hardening: Actions are pinned to version tags instead of commit SHAs. Permissions are otherwise minimal (permissions: {} with per-job contents: read), and persist-credentials: false is set.
✅ Input validation: Not applicable beyond what the snippets already do.
✅ Error handling: A real improvement. Snippets now exit non-zero and write errors to stderr. This also fixes the redo_with_info_continuous.py TypeError, where responses_message() was called without its required arguments.
✅ Sensitive data in logs: None.
✅ License files: No .lic files and no AQAAAD strings are present in the diff.

Summary

The main item to fix is the potential hang in `add_queue.py` when the consumer shuts down early. Secondary items are the inconsistent workflow triggers and the CHANGELOG release and version handling.

Automated code review analyzing defects and coding standards

github-actions · 2026-10-09T19:47:18Z

Super-linter summary

Language	Validation result
CHECKOV	Pass ✅
GITHUB_ACTIONS	Pass ✅
GITHUB_ACTIONS_ZIZMOR	Pass ✅
GITLEAKS	Pass ✅
GIT_MERGE_CONFLICT_MARKERS	Pass ✅
JSCPD	Pass ✅
PRE_COMMIT	Pass ✅
SPELL_CODESPELL	Pass ✅
TRIVY	Pass ✅
YAML	Pass ✅
YAML_PRETTIER	Pass ✅

All files and directories linted successfully

For more information, see the GitHub Actions workflow run

Powered by Super-linter

- Remove the darwin and windows snippet workflows and their harness code: the Python SDK isn't shipped for macOS or Windows - Replace the private executor._max_workers with an explicit MAX_WORKERS (8, matching Java) in the futures and queue snippets - Bandit CI now uses pyproject.toml, so B101 is skipped as configured - Close the SQLite connection explicitly in tests/build_repo.py - Keep CHANGELOG entries under Unreleased; set pyproject version to 0.0.10 - Document pytest and correct flake8/bandit commands in .claude/CLAUDE.md

github-actions · 2026-10-09T20:16:02Z

🤖 Claude Code Review

PR Review: Python snippet tests, CI and exit codes

Code Quality

❌ pyproject.toml:3: the version drops from 1.2.8 to 0.0.10. This looks accidental. It is unrelated to the PR and goes backwards. CHANGELOG.md still treats 0.0.10 as the last release, so check which value is right. Revert it unless the change is intentional.
✅ Style is consistent and the code matches the surrounding snippets. Each snippet that now calls sys.exit(1) imports sys, which I checked.
✅ No commented-out code. Names are meaningful, for example MAX_WORKERS replaces the private executor._max_workers.
⚠️ DRY: MAX_WORKERS = 8 and the error handling repeat across snippets. That is acceptable, since each snippet is meant to be standalone.
⚠️ Defect (minor), python/loading/add_queue.py:92-98. When consumer() raises, the with open(INPUT_FILE) block closes the file while the daemon producer thread may still be reading it. The thread can then fail with ValueError: I/O operation on closed file, and that traceback may reach stderr. The exit code is unaffected and the process still exits with 1. Consider having the producer catch ValueError, or opening the file inside the producer.
✅ The add_queue.py redesign is sound. It uses a None sentinel instead of queue.empty(), which removes the early-stop race. It also drops multiprocessing, so the engine is no longer shared across processes.
✅ The redo_with_info_continuous.py bug fix is correct. responses_message() was called without its required arguments and raised a TypeError.
✅ .claude/CLAUDE.md has no environment-specific content. The new paths in it are the standard defaults.

Testing

✅ python/tests adds an end-to-end test for every snippet. It discovers snippets automatically, and test_snippet_table_is_current catches stale entries in the SNIPPETS table.
✅ Edge cases are covered: expected errors (SENZ7221, SENZ2207), ctrl-c handling, stdin confirmation and timeouts.
✅ Subprocess isolation keeps Senzing out of the pytest process. Each test gets a private copy of its template repository.
⚠️ I can't confirm the 80% coverage target, because the tests run snippets as subprocesses and in-process coverage won't see them. Treat that item as not applicable.
⚠️ abstract_factory_parameters.py is skipped because it hardcodes its own settings. This is documented in the entry.

Documentation

✅ python/README.md documents running the tests and the environment variables.
✅ CHANGELOG.md has Added, Changed and Fixed entries.
✅ .vscode/cspell.json is updated for the new words.
⚠️ Run prettier on the Markdown. The long line in python/README.md and the wrapped CHANGELOG.md lines should follow the repo's prettier settings.

Security

✅ No hardcoded credentials and no .lic files. No string starting with AQAAAD appears.
✅ The # nosec B603 and B404 suppressions are justified. The subprocesses only run sys.executable on repository files.
✅ Error handling improves. Errors go to stderr with exit status 1.
✅ The workflow sets permissions: {} at the top level and contents: read on the job. It also uses persist-credentials: false.
⚠️ Workflow actions are pinned by version tag, not commit SHA. This is consistent with the rest of the repo, but SHA pins are stronger.
✅ .gitignore is correct. resources/output/* with !resources/output/.gitkeep keeps the folder in the repo.

Summary

The PR is solid overall. Fix the `pyproject.toml` version regression before merging. Optionally harden the `add_queue.py` producer against the file closing early.

Automated code review analyzing defects and coding standards

v1 (= v1.0.0) installs bandit without the toml extra on Python 3.8, so '-c pyproject.toml' failed and produced no SARIF to upload.

…l error The main thread closes the input file when the consumer raises, which could interrupt the producer mid-read. Ignore that ValueError only when the file has been closed, so decode errors are still raised.

github-actions · 2026-10-09T20:23:07Z

🤖 Claude Code Review

PR Review

Code Quality

✅ Style: The changes are consistent with the surrounding code. The MAX_WORKERS = 8 constant replaces the private executor._max_workers access everywhere. Line wrapping in some hunks differs only because of black at 120 columns, e.g. python/loading/add_futures.py and python/information/get_stats.py.
✅ No commented-out code.
✅ Variable names: Names are clear. queue was renamed to record_queue, so it no longer shadows the queue module.
⚠️ DRY: MAX_WORKERS = 8 is copied into about 8 snippets. That is acceptable for stand-alone snippets. The except SzError … sys.exit(1) tail is repeated too, which is expected here.
❌ Defect: unrelated version downgrade. pyproject.toml changes version = "1.2.8" to "0.0.10". This is a regression unrelated to the PR's purpose and looks like a merge or rebase artifact. Revert it unless it is intentional, and if it is, say so in the CHANGELOG.
⚠️ python/loading/add_queue.py (producer and consumer):
- After a fatal error the main thread exits the with open(...) block and closes the file. The daemon producer may still be blocked in record_queue.put(...) on the full queue (maxsize=200) and in the finally: put(None). It is a daemon, so the process still exits, and the ValueError/closed handling is sensible.
- If the producer fails with a non-ValueError exception, the exception is only printed by the thread machinery. The consumer still gets the None sentinel and exits 0 with a partial load. Consider recording the failure and exiting non-zero.
⚠️ python/redo/redo_continuous_futures.py:~111: while len(futures) < MAX_WORKERS: if record := get_redo_record(engine): … busy-loops when no record is available. This was already there, but it is worth a follow-up.
✅ redo_with_info_continuous.py: This fixes a real bug, a call to responses_message() without its required arguments that raised TypeError. Removing the dead function and calling sys.exit(1) is correct.
✅ sys.exit(1) additions: Each modified snippet either already imported sys for its mock_logger or has the import added in the diff. I did not run them.
✅ .claude/CLAUDE.md: It contains only generic commands and the default /opt/senzing paths, with no developer-specific content. The added testing section is appropriate.

Testing

✅ New integration harness: python/tests runs every snippet end to end. Snippets are auto-discovered, SNIPPETS holds the exceptions, and test_snippet_table_is_current guards against stale entries.
✅ Edge cases covered: The tests cover expected-failure snippets (SENZ7221), stderr-noise checks, ctrl-c snippets, stdin prompts, and the error-input file (SENZ2207).
⚠️ conftest.py: signal.SIGINT and signal.pause() snippets are Linux and macOS only. The CI workflow is Linux-only, but the README doesn't say Windows is unsupported.
⚠️ Runtime: Three redo snippets use sigint_after=30, so they add about 90 s of fixed wall time. This is acceptable, but consider shortening it.
⚠️ No coverage measurement: This is expected for snippets, and the 80% target isn't applicable.
⚠️ No direct unit tests for the new add_queue.py logic: submit_next, the producer sentinel, and the fatal-error shutdown path are only covered by the happy-path end-to-end run.

Documentation

✅ CHANGELOG.md: Added, Changed and Fixed entries are present and accurate.
✅ python/README.md: It documents how to run the tests and the environment overrides.
✅ Inline comments: They are present where the logic is non-obvious, e.g. the producer's ValueError handling and the SNIPPETS entries.
⚠️ Markdown style: python/README.md uses long unwrapped lines. This is valid under prettier's default proseWrap: preserve, but it is inconsistent with the CHANGELOG, which wraps at 120.
✅ cspell: New words were added to .vscode/cspell.json.

Security

✅ No hardcoded credentials: The SQLite URL sqlite3://na:na@… is a placeholder for a temporary test DB.
✅ No .lic files or AQAAAD strings appear in the diff.
✅ Subprocess use: It only runs sys.executable on repo files, with # nosec B603/B404 and a justification comment. There is no shell invocation.
✅ Error handling: Errors now go to stderr with a non-zero exit code instead of being swallowed with exit 0.
⚠️ CI workflow: It uses permissions: {} at the top level and contents: read on the job, and persist-credentials: false, which is good. The actions/checkout@v7.0.1, actions/setup-python@v7.0.0 and bandit-action@v1.0.1 pins are plausible, but I couldn't verify they exist (no network). Consider SHA pinning, as the repo's other workflows do, if that is the convention.
✅ No sensitive data in logs. settings_for builds the connection string but doesn't print it. Failure output only shows snippet stdout and stderr.

Summary

The PR is solid overall. One item should be fixed before merge: the `pyproject.toml` version regression from 1.2.8 to 0.0.10. The producer-failure exit code in `add_queue.py` is worth tightening, and the rest are minor suggestions.

Automated code review analyzing defects and coding standards

An error reading the input (for example a UnicodeDecodeError) was only printed by the thread machinery, and the snippet exited 0 after a partial load. The producer now hands the error to the main thread, which raises it.

… records wait Refilling the pool looped on get_redo_record() until MAX_WORKERS records were available. Stop at the first empty fetch, and pause if nothing was fetched so wait() isn't called on an empty set.

github-actions · 2026-10-09T20:33:05Z

🤖 Claude Code Review

PR Code Review

Overall this is a good PR. It adds an end-to-end test harness for the Python snippets, fixes real exit-code and error-handling bugs, and replaces the private _max_workers access with MAX_WORKERS. I found one likely unintended change and a few smaller concerns. I reviewed the diff only and did not run the code or the tests.

Code Quality

✅ Style conventions. The code is consistent with black at 120 columns, and the new tests use type hints and dataclasses. Variable names are meaningful (record_queue, more_records, producer_errors).
✅ No commented-out code. The dead responses_message() helper in redo_with_info_continuous.py was removed. It was also the source of the TypeError, because it was called with no arguments.
✅ DRY. The repeated MAX_WORKERS = 8 and sys.exit(1) lines are acceptable because each snippet is meant to stand alone. In the tests, Repo, Snippet and SNIPPETS keep the per-snippet configuration in one place.
❌ Defects and risks:
- pyproject.toml:3 changes version from 1.2.8 to 0.0.10. This is a downgrade that has nothing to do with this PR, and it isn't mentioned in the changelog. It may be an accidental revert or a bad merge, so please confirm or drop it. If it is intentional, it should be explained.
- python/loading/add_queue.py, around the producer_thread.join() call:
  - If consumer() returns normally after setting shutdown and the producer is blocked on record_queue.put() with the queue full (maxsize=200), join() never returns. That would happen with the 500-record file.
  - If the error path raises instead, the daemon thread is simply abandoned, which is fine.
  - Please confirm that every shutdown path raises or exits. If one doesn't, drain the queue or use a join timeout.
- python/loading/add_queue.py, in producer(): the finally: record_queue.put(None) can also block if nobody is consuming. That is harmless only because the thread is a daemon.
- python/initialization/purge_repository.py and python/loading/add_records_loop.py: the diff adds sys.exit(1) without adding import sys. These files probably already import it, since mock_logger writes to sys.stderr. Please verify, because a missing import would only show up on the error path.
- python/redo/redo_continuous_futures.py: the new if not futures: redo_paused = True branch is subtle. A short comment on why it avoids the spin would help.
✅ .claude/CLAUDE.md. The edits are general. The /opt/senzing/... paths are the standard install defaults rather than anything specific to one developer's machine.

Testing

✅ Coverage. python/tests runs every discovered snippet end to end against its own temporary SQLite repository. This covers the exit-code changes, stderr cleanliness and the expected-error cases.
✅ Edge cases. The tests cover the ctrl-c snippets, stdin-driven snippets, expected failures (SENZ7221, SENZ2207) and skipped snippets.
❌ Gaps:
- There are no targeted tests for the add_queue.py producer-error path or for the redo_continuous_futures empty-queue fix. Those are the two bug fixes the changelog highlights.
- test_snippet_table_is_current checks only one direction. A snippet that needs special setup but has no SNIPPETS entry is caught only when it fails.
- No coverage figure is reported. Coverage isn't very meaningful for standalone scripts run as subprocesses.

Documentation

✅ python/README.md and .claude/CLAUDE.md document how to run the tests.
✅ CHANGELOG.md has Added, Changed and Fixed entries.
❌ The changelog doesn't mention the pyproject.toml version change.
✅ Inline comments explain the non-obvious logic: the producer's error handoff, the SNIPPETS table entries and the bandit nosec suppressions.
✅ The markdown reads as CommonMark-compliant and fits prettier formatting. I didn't run prettier.

Security

✅ No hardcoded credentials. The SQLite connection string in the tests uses a placeholder (na:na) and a temporary path.
✅ No license files. I found no .lic files and no strings starting with AQAAAD.
✅ Error handling. Errors now go to stderr and the process exits non-zero, which is a clear improvement over printing to stdout and exiting 0.
✅ No sensitive data in logs.
✅ Subprocess use. The subprocess calls only run sys.executable on repository files, and the # nosec comments explain why.
❌ Workflow versions. .github/workflows/python-linux-snippets.yaml uses actions/checkout@v7.0.1 and actions/setup-python@v7.0.0. Please verify these tags exist. Consider pinning third-party actions to commit SHAs, as the bandit workflow does with its explicit version.
✅ The workflow sets permissions: {} at the top level and contents: read per job, and uses persist-credentials: false.

Summary

The only blocking item is the unexplained pyproject.toml version downgrade. Before merging, please also:

confirm the add_queue.py shutdown path can't hang on join();
verify the sys imports in the files noted above;
verify the pinned action versions.

Automated code review analyzing defects and coding standards

github-actions · 2026-10-09T20:55:51Z

🤖 Claude Code Review

Review

Code Quality

✅ Style: The changes match the existing snippet style. The MAX_WORKERS constant replaces the private executor._max_workers, which is an improvement.
✅ No commented-out code.
✅ Variable names: record_queue no longer shadows the queue module, and submit_next is clear.
❌ DRY: The same sys.exit(1) and mock_logger boilerplate is repeated in every snippet. That is acceptable for standalone teaching snippets. Snippet, Repo and the table-driven tests are well factored.
❌ Defects and edge cases:
- pyproject.toml:3: the version goes from 1.2.8 back to 0.0.10. This looks unintended and has nothing to do with the PR's purpose. If it's deliberate, explain it. If not, revert it, because it is a version downgrade.
- python/loading/add_queue.py:~93-107 (producer/consumer shutdown): the path is safe today, but it is fragile.
  - After a fatal error the consumer raises, so producer_thread.join() is skipped. The producer may stay blocked on record_queue.put() on the full queue, which holds 200 records. It only exits cleanly because the thread is a daemon.
  - If the consumer ever returned early without raising, join() would hang forever. A short comment, or draining the queue on shutdown, would make this robust.
- python/tests/test_snippets.py, redo_* snippets: the tests depend on a fixed sigint_after=30. This is timing-based and could be flaky on slow CI, though there is a 30s grace period for the interrupt.
- python/tests/conftest.py:senzing_paths: CONFIGPATH defaults to /etc/opt/senzing with no SENZING_PATH relative fallback. This is fine, and it is documented in the README.
- python/redo/redo_continuous_futures.py: the spin fix (while len(futures) < MAX_WORKERS and (record := get_redo_record(engine)), then redo_paused = True if nothing was submitted) looks correct.
- The redo_with_info_continuous.py fix removes the responses_message() call, which would have raised a TypeError. The removal is correct.
- Every snippet that gained sys.exit(1) already imports sys, or the PR adds the import. Please confirm initialization/purge_repository.py and signal_handler.py import sys, since the diff doesn't show it.
✅ .claude/CLAUDE.md: It contains no local-environment-specific content. The paths under /opt/senzing are standard install defaults, and the doc is general.

Testing

✅ Every snippet is run end to end, and a test fails if a snippet is added that the harness can't handle. test_snippet_table_is_current guards against stale entries.
❌ Coverage: There are no unit tests for the changed logic in add_queue.py, such as producer error propagation, the None sentinel, or early shutdown. The smoke test only covers the happy path. The error paths (returncode=1 for config ID, expect_stderr) are covered for a few snippets only.
❌ No coverage measurement is configured, so the >80% target can't be verified. That is understandable for snippets.

Documentation

✅ python/README.md, CLAUDE.md and CHANGELOG.md are updated, with Added, Changed and Fixed entries.
✅ Inline comments explain the non-obvious logic (thread error hand-off, closed file).
⚠️ The Markdown looks prettier-compatible. The README's long lines are fine for prose, but the CHANGELOG wraps at 120 characters. Run prettier --check to confirm.
⚠️ The CHANGELOG doesn't mention the CI workflow pinning (bandit-action@v1.0.1, configfile). This is minor.

Security

✅ No hardcoded credentials. The sqlite3://na:na@ connection string is a placeholder for a throwaway database.
✅ No .lic files and no AQAAAD strings appear in the diff.
✅ The subprocess calls only run sys.executable on repository files, and the # nosec comments justify this.
✅ Error handling is improved. Errors go to stderr with a non-zero exit code instead of being printed to stdout with exit 0.
✅ The workflow uses permissions: {} at the top level, and contents: read per job. It also uses persist-credentials: false.
⚠️ The action pins are version tags rather than commit SHAs (actions/checkout@v7.0.1, setup-python@v7.0.0, senzing-factory/...@v4/@v5). SHA pinning would be stricter if that is your policy.
✅ No sensitive data is logged.

Summary

I found no blocking defects. The one item to resolve before merging is the unexplained `pyproject.toml` version downgrade from `1.2.8` to `0.0.10`. The other points are the fragile producer-thread shutdown in `add_queue.py`, the timing-dependent redo tests, and the lack of unit tests for the new `add_queue.py` logic.

Automated code review analyzing defects and coding standards

github-actions · 2026-10-09T21:09:30Z

🤖 Claude Code Review

Review summary

I found no blocking defects. Code review was by reading only: I did not run the test suite, linters or CI. A few non-blocking items are below.

Code Quality

✅ Style: The changes follow the repo's black, isort and flake8 settings. sys is imported wherever sys.exit(1) was added. Checked directly in purge_repository.py.
✅ No commented-out code. This includes the removal of the dead responses_message(). It was also broken, since redo_with_info_continuous.py called it with no arguments, which raised a TypeError.
✅ Naming: MAX_WORKERS replaces the private executor._max_workers. record_queue stops queue shadowing the module.
⚠️ DRY: MAX_WORKERS = 8 and the identical except SzError … sys.exit(1) tail are repeated across snippets. That is acceptable for standalone teaching snippets.
✅ Defects checked:
- python/loading/add_queue.py:94-110
  - If the consumer raises after a fatal error, the exception skips producer_thread.join().
  - The with open closes the file, and the daemon producer cannot hang the exit even if it is blocked on a full queue.
  - The in_file.closed guard stops the resulting ValueError from being recorded as a producer error.
- python/redo/redo_continuous_futures.py:111-114
  - The new while len(futures) < MAX_WORKERS and (record := …) loop fixes the spin when fewer redo records than workers are waiting.
  - if not futures: redo_paused = True correctly goes back to pausing.
- python/loading/add_queue.py:107-108 re-raises the producer error. A non-SzError such as OSError will surface as a traceback with a non-zero exit. That is reasonable, but note it is not routed through mock_logger.
✅ .claude/CLAUDE.md: It contains only general commands. The /opt/senzing paths are the documented defaults, not machine-specific.

Testing

✅ Integration: python/tests runs every snippet end to end, discovered automatically, with an exit-code and clean-stderr check.
✅ Edge cases: The SNIPPETS table covers error cases, stdin and SIGINT, and test_snippet_table_is_current guards against stale entries.
⚠️ Gaps:
- The new snippet tests do not exercise the fixed edge cases directly. These are add_queue failing when the producer fails, the queue running empty early, and redo_continuous_futures with fewer redo records than workers. The tests would not reliably catch a regression in these.
- No coverage figure is reported. That is unlikely to be meaningful for snippets.
- abstract_factory_parameters.py is skipped because it hardcodes its settings.

Documentation

✅ README, CLAUDE.md and CHANGELOG are updated. The changelog entry is dated 2026-10-09 and has Added, Changed and Fixed sections.
⚠️ Version: pyproject.toml goes from 1.2.8 to 0.0.11. This is consistent with the changelog, but it is a version decrease. Confirm it is intended.
⚠️ Markdown: The python/README.md paragraphs and the CLAUDE.md testing paragraph are long single lines, while CHANGELOG.md wraps at 120. Run prettier to confirm the repo's proseWrap setting, and check for trailing whitespace.
✅ Inline comments explain the non-obvious thread and file-close handling.

Security

✅ No hardcoded credentials. abstract_factory_parameters.py already had a placeholder SQLite path, and it is skipped in tests.
✅ Errors now go to stderr with exit code 1.
✅ No .lic files and no strings starting with AQAAAD.
✅ Subprocess use runs only sys.executable on repo paths, with # nosec B603/B404 annotations.
⚠️ CI workflow: .github/workflows/python-linux-snippets.yaml uses version tags such as actions/checkout@v7.0.1 and setup-python@v7.0.0. Confirm those tags exist. Consider pinning third-party actions to commit SHAs. Permissions are least-privilege (permissions: {} at the top, then contents: read on the job) and persist-credentials: false is set.

Automated code review analyzing defects and coding standards

antaenc requested review from a team as code owners October 9, 2026 19:42

github-advanced-security Bot found potential problems Oct 9, 2026

View reviewed changes

antaenc changed the title ~~feat(python): add snippet tests and CI for Linux, macOS and Windows~~ feat(python): add snippet tests and Linux CI Oct 9, 2026

antaenc added 2 commits October 9, 2026 16:17

fix(ci): pin bandit-action v1.0.1 so pyproject.toml config can be read

7a2ae58

v1 (= v1.0.0) installs bandit without the toml extra on Python 3.8, so '-c pyproject.toml' failed and produced no SARIF to upload.

fix(python): avoid closed-file traceback in add_queue.py after a fata…

c1207ae

…l error The main thread closes the input file when the consumer raises, which could interrupt the producer mid-read. Ignore that ValueError only when the file has been closed, so decode errors are still raised.

antaenc added 2 commits October 9, 2026 16:27

fix(python): redo_continuous_futures.py no longer spins when few redo…

d15bd06

… records wait Refilling the pool looped on get_redo_record() until MAX_WORKERS records were available. Stop at the first empty fetch, and pause if nothing was fetched so wait() isn't called on an empty set.

Merge branch 'main' into 11-ant-1

b04b2a3

kernelsam approved these changes Oct 9, 2026

View reviewed changes

chore(release): 0.0.11

8a0af0f

antaenc merged commit 1f55af7 into main Oct 9, 2026
84 checks passed

antaenc deleted the 11-ant-1 branch October 9, 2026 21:19

antaenc mentioned this pull request Oct 9, 2026

Add linting, testing, etc to workflows and make for Python #11

Closed

Navigation Menu

Repository navigation

feat(python): add snippet tests and Linux CI - #248

feat(python): add snippet tests and Linux CI#248
antaenc merged 8 commits into
mainfrom
11-ant-1

antaenc commented Oct 9, 2026 •

edited by github-actions Bot

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Oct 9, 2026

Uh oh!

github-actions Bot commented Oct 9, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Oct 9, 2026

Uh oh!

github-actions Bot commented Oct 9, 2026

Uh oh!

github-actions Bot commented Oct 9, 2026

Uh oh!

github-actions Bot commented Oct 9, 2026

Uh oh!

github-actions Bot commented Oct 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

antaenc commented Oct 9, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Fixes found along the way

Housekeeping

Testing

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Oct 9, 2026

🤖 Claude Code Review

Code Review

Code Quality

Testing

Documentation

Security

Summary

The main item to fix is the potential hang in add_queue.py when the consumer shuts down early. Secondary items are the inconsistent workflow triggers and the CHANGELOG release and version handling.

Uh oh!

github-actions Bot commented Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Super-linter summary

Uh oh!

github-actions Bot commented Oct 9, 2026

🤖 Claude Code Review

PR Review: Python snippet tests, CI and exit codes

Code Quality

Testing

Documentation

Security

Summary

The PR is solid overall. Fix the pyproject.toml version regression before merging. Optionally harden the add_queue.py producer against the file closing early.

Uh oh!

github-actions Bot commented Oct 9, 2026

🤖 Claude Code Review

PR Review

Code Quality

Testing

Documentation

Security

Summary

The PR is solid overall. One item should be fixed before merge: the pyproject.toml version regression from 1.2.8 to 0.0.10. The producer-failure exit code in add_queue.py is worth tightening, and the rest are minor suggestions.

Uh oh!

github-actions Bot commented Oct 9, 2026

🤖 Claude Code Review

PR Code Review

Code Quality

Testing

Documentation

Security

Summary

Uh oh!

github-actions Bot commented Oct 9, 2026

🤖 Claude Code Review

Review

Code Quality

Testing

Documentation

Security

Summary

Uh oh!

github-actions Bot commented Oct 9, 2026

🤖 Claude Code Review

Review summary

Code Quality

Testing

Documentation

Security

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

antaenc commented Oct 9, 2026 •

edited by github-actions Bot

Loading

The main item to fix is the potential hang in `add_queue.py` when the consumer shuts down early. Secondary items are the inconsistent workflow triggers and the CHANGELOG release and version handling.

github-actions Bot commented Oct 9, 2026 •

edited

Loading

The PR is solid overall. Fix the `pyproject.toml` version regression before merging. Optionally harden the `add_queue.py` producer against the file closing early.

The PR is solid overall. One item should be fixed before merge: the `pyproject.toml` version regression from 1.2.8 to 0.0.10. The producer-failure exit code in `add_queue.py` is worth tightening, and the rest are minor suggestions.