Skip to content

SEP-1557: _dsn_safe delimiter guard in checksums/models.py is a near-verbatim copy of archives/models.py's _dsn_safe - #1128

Merged
yyyyyyyan merged 10 commits into
mainfrom
SEP-1557
Jul 15, 2026
Merged

SEP-1557: _dsn_safe delimiter guard in checksums/models.py is a near-verbatim copy of archives/models.py's _dsn_safe#1128
yyyyyyyan merged 10 commits into
mainfrom
SEP-1557

Conversation

@peter-o-addo

@peter-o-addo peter-o-addo commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Summary

Extract the duplicated _dsn_safe delimiter guard into a shared dsn_safe helper in app/core/utils/fields.py so checksums, archives, and alters reuse one validator for free-typed schema/table/host names.

  • app/core/utils/fields.py : add shared dsn_safe(value: str) -> str that rejects , and = with a unified "DSN delimiters" error message
  • app/sep/apps/checksums/models.py, app/sep/apps/archives/models.py, app/sep/apps/alters/models.py : remove local _dsn_safe copies; import and call the shared helper from existing field_validator wrappers (ints and None still pass through unchanged)
  • tests/app/core/utils/test_fields.py : add TestDsnSafe unit tests for the shared helper; existing checksums, archives, and alters model/schema tests still pass

Tested

  • Checksums (/apps/checksums/new): After filling Task Name, Execution Host, and Database Host, typing bad,name in Databases blocks submit with "DSN delimiters", while mydb (or an inventory pick) saves OK.
  • Archives (/apps/archives/new): With source src_db/src_tbl, destination table dst_tbl, and WHERE id < 100, typing a,b in Destination Schema shows "DSN delimiters", while leaving Destination Schema empty saves OK.
  • Alters (/schema-change/alters/new): With task, executor, MySQL service, and alter ADD COLUMN x INT filled, typing a=b in Schema (or bad,name in Table) shows "DSN delimiters", while Schema app and Table users save OK.

Checklist

  • New/modified functions have type hints and rST docstrings
  • New tests added for new features or bug fixes
  • All tests pass locally (make test)
  • Pre-commit hooks pass (make run-pre-commit)
  • Database migrations generated if models changed (make makemigrations)
  • User-facing changes documented (README, inline help, UI text)
  • Configuration changes documented with examples
  • Changelog fragment added under changelog.d/ if the change is user-facing (make changelog-add), or confirmed N/A (internal-only change, or a same-release-cycle fix for an unreleased sibling ticket)

@peter-o-addo
peter-o-addo marked this pull request as ready for review July 15, 2026 12:04
Copilot AI review requested due to automatic review settings July 15, 2026 12:04
@peter-o-addo peter-o-addo added the qa in progress Someone is currently testing this PR - do not merge it label Jul 15, 2026
@peter-o-addo

Copy link
Copy Markdown
Contributor Author
image image image

@peter-o-addo peter-o-addo added qa passed Tests for this PR are completed and successful. and removed qa in progress Someone is currently testing this PR - do not merge it labels Jul 15, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR extracts a duplicated “DSN delimiter” validator (_dsn_safe) from several SEP app models into a shared dsn_safe() helper in app/core/utils/fields.py, and updates the checksums/archives/alters apps to use the shared implementation, with new unit tests covering the helper.

Changes:

  • Add dsn_safe(value: str) -> str to app/core/utils/fields.py to reject , and = with a unified error message.
  • Remove per-app _dsn_safe copies and switch validators in checksums/archives/alters models to call the shared helper.
  • Add unit tests in tests/app/core/utils/test_fields.py for the new helper.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
app/core/utils/fields.py Adds shared dsn_safe() helper for DSN/CLI delimiter guarding.
app/sep/apps/checksums/models.py Replaces local _dsn_safe with shared dsn_safe in list validators.
app/sep/apps/archives/models.py Replaces local _dsn_safe with shared dsn_safe in destination schema/host validators.
app/sep/apps/alters/models.py Replaces local _dsn_safe with shared dsn_safe in schema/table validators.
tests/app/core/utils/test_fields.py Adds focused unit coverage for dsn_safe().

Comment thread app/core/utils/fields.py Outdated
@github-actions

github-actions Bot commented Jul 15, 2026

Copy link
Copy Markdown

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  app/core/utils
  fields.py
  app/sep/apps/alters
  models.py
  app/sep/apps/archives
  models.py
  app/sep/apps/checksums
  models.py
  app/sep/sync/syncers
  pmm.py
  app/sep/sync/syncers/mysql
  syncer.py
Project Total  

This report was generated by python-coverage-comment-action

@yyyyyyyan
yyyyyyyan enabled auto-merge (squash) July 15, 2026 17:32
@yyyyyyyan
yyyyyyyan merged commit 9eb822e into main Jul 15, 2026
17 checks passed
@yyyyyyyan
yyyyyyyan deleted the SEP-1557 branch July 15, 2026 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python qa passed Tests for this PR are completed and successful.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants