Skip to content

Added creation of test users which was previously included in the image#972

Merged
ToreMerkely merged 4 commits into
mainfrom
add-cli-test-users
Jun 25, 2026
Merged

Added creation of test users which was previously included in the image#972
ToreMerkely merged 4 commits into
mainfrom
add-cli-test-users

Conversation

@ToreMerkely

Copy link
Copy Markdown
Contributor

No description provided.

@ToreMerkely ToreMerkely enabled auto-merge (squash) June 25, 2026 05:28
@claude

claude Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Claude finished @ToreMerkely's task in 1m 28s —— View job


Review: PR #972 — standalone test-user creation

  • Gather context (read changed files)
  • Static analysis — N/A (no Go files changed; only Python/shell/.snyk)
  • Code review
  • Post feedback

This is a clean, well-scoped change that moves the previously image-baked test-user seeding into the CLI repo. I verified the wiring end-to-end and it all lines up. The two earlier review comments have been addressed (keyword args on add_api_key, explanatory comment on the fixed auth_token).

Wiring is consistent ✅

  • docker-compose.yml mounts ./server-scripts:/app/test:ro.
  • bin/reset-or-start-server.sh now execs /app/test/create_standalone_test_users.py (and the pre-existing /app/test/clean_database.py) — both paths match the mount, replacing the old /demo/... image path.
  • Both scripts are present in server-scripts/ with the exec bit set (-rwxr-xr-x), so the bind-mounted shebang scripts run fine even under the :ro mount.

Good work 👍

  • Clear header comment in create_standalone_test_users.py explaining the PYTHONPATH=/app/src / lib+model cross-repo dependency — genuinely helpful for a future reader.
  • The *"@sha256:"* guard in restart_server() cleanly separates the remote (digest-pinned, needs AWS login + pull) flow from the locally-built merkely-tag flow, with a comment explaining why.
  • The .snyk exclusion is well-documented (fake test data, links to kosli-dev/server#5479) rather than a bare suppression.

Minor / optional observations (non-blocking)

  1. bin/reset-or-start-server.sh:29 — the remote-vs-local heuristic keys off @sha256:. That's correct for the current convention (remote = digest-pinned), but a remote image referenced by tag (not digest) would silently skip the AWS login/pull. Fine given today's workflow; just worth keeping in mind if the image-reference convention ever changes.
  2. server-scripts/create_standalone_test_users.py:38-41completed_signup, the API key, and auth_token are mutated on the user object without an explicit .save() in this script. Since this is a byte-faithful port of the working image-baked script and make test_setup seeds all four users successfully, persistence clearly happens inside the lib/model layer — no action needed, noting only because the persistence point isn't visible from this repo.

Nothing here blocks merge. Nice cleanup. 🚢

Comment thread server-scripts/create_standalone_test_users.py
Comment thread server-scripts/create_standalone_test_users.py Outdated
ToreMerkely and others added 3 commits June 25, 2026 07:34
The hardcoded auth_token is fake test data used to seed the local
integration-test server, not a real credential.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
@ToreMerkely ToreMerkely merged commit 5f2a689 into main Jun 25, 2026
20 checks passed
@ToreMerkely ToreMerkely deleted the add-cli-test-users branch June 25, 2026 06:46
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.

2 participants