Skip to content

Commit 6fe9527

Browse files
authored
feat(sqlserver): support client certificates on the ODBC connection string (#6)
* perf(sqlserver): resolve backend exports lazily so expressions import without a driver Every backend __init__ imported all of its submodules eagerly, and .backend imports the DB driver. Python executes a parent package's __init__ before any submodule, so `impl.sqlserver.expression` could not be imported without the driver present - even though no expression class touches it. Auditing the expression namespace needed a working driver; pyodbc additionally needs the OS libodbc.so, so a plain pip install was not enough and sqlserver dropped out of the audit entirely. Public names are now resolved on first access through a PEP 562 module __getattr__, with the name -> (module, attribute) map derived from the existing import statements. The exported surface is unchanged: attribute access, __all__, dir() and `from impl.sqlserver import *` all behave as before, and the previous eager imports move into an `if TYPE_CHECKING:` block so mypy and IDEs still resolve them. The existing __getattr__ is preserved as _optional_dependency_getattr, so the async backends keep raising an install hint for aioodbc instead of a bare ImportError. Importing the package, or its .expression submodule, no longer loads the driver; the driver error now surfaces at first use of the name that needs it, where it is actionable. 55 public names verified to resolve. * refactor(backend): make impl.<backend> an open package Drop the top-level impl/<backend>/__init__.py so the directory becomes a PEP 420 namespace portion and no longer re-exports every public name. Each name is now imported from the sub-package that defines it, so a symbol has one public path instead of two. The facade also made "expression is importable on its own" unenforceable: importing impl.<backend>.expression executed the parent __init__, which pulled in the driver. With the file gone, the sub-packages stand alone by construction rather than by a lazy __getattr__ that each backend had to maintain separately. This is a breaking change for anyone importing `from rhosocial.activerecord.backend.impl.<backend> import X`; the replacement is `...impl.<backend>.<module> import X`. * fix(providers): resolve the async backend across sibling modules The provider gave the worker tasks a single module path and the task code looked for the async counterpart one level up. That was enough while the sync and async classes shared the impl.<backend> facade; with the facade gone they live in different modules and the one-level lookup fell through to the sync class, so an async worker would silently run the sync backend. Walk up the chain first, then try the backend's own async_backend module. No package scanning is involved: both candidates are named explicitly. * refactor(backend): keep both backend classes in impl.<backend>.backend Move the sync and async classes into a backend/ package with the fixed layout: backend/backend.py and backend/async_backend.py. Eight backends kept them as two modules beside backend/, and sqlite and postgres used backend/sync.py, so the class locations differed three ways between backends. With the layout fixed the worker tasks derive the async class from the sync module's own package instead of searching for it, so the _find_async_backend helper is gone. * fix(backend): correct relative import depth inside backend/ The move into backend/ shifted every relative import by one level, and the function-local ones were missed, so impl.bigquery could not reach its adapters module. Resolve each target against the filesystem instead of counting dots. * refactor(backend): re-export the async class from the backend package impl.<backend>.backend already re-exported the sync class, so leaving the async one out made the two classes reachable by different rules. Export both. The eight backends whose async driver is a hard requirement import it eagerly. SQLite and SQL Server keep a PEP 562 __getattr__, because aiosqlite and aioodbc ship as the [async] extra and importing either eagerly would make the async class a requirement for sync-only users. * fix(test): patch the backend class on the module the CLI reads The CLI tests monkeypatched SQLServerBackend on impl.sqlserver, which no longer carries the name now that the directory is a namespace. Patch impl.sqlserver.backend instead, and the async class on impl.sqlserver.backend.async_backend, which is where each is defined. * fix(test): patch the async backend class on the module the CLI reads The remaining multi-line monkeypatch still targeted impl.sqlserver.backend for AsyncSQLServerBackend, but the CLI imports it from impl.sqlserver.backend.async_backend inside the handler, so the patch never reached it and the real backend was constructed. * fix(sqlserver): repair the import line the package move truncated The two CLI demos embed runnable Python as a string literal, and the import migration rewrote the module path inside it without keeping the statement terminator, leaving the string unterminated: "from rhosocial.activerecord.backend.impl.oracle.backend.async_backend import OracleBackend ast.parse rejects all four demo modules with "unterminated string literal", so they could not even be imported, let alone run. The migration also dropped the trailing OracleConnectionConfig from the original single-statement import; the surrounding lines already import it from its own module, so restoring the ";" recovers the intended statements. The SQL Server demos additionally reached SQLServerConnectionConfig through the impl.sqlserver facade, which no longer exists now that the package is open, so they now name impl.sqlserver.config like every other backend. Verified: ast.parse is clean across every src/ and tests/ module in both repos. * chore: remove two legacy re-export shims shadowed by same-named packages mixins.py and protocols.py each sat next to a package of the same name, holding nothing but a re-export of that package's contents. A regular package takes precedence over a same-named module, so both shims were unreachable: importing impl.sqlserver.mixins or impl.sqlserver.protocols always resolved to the package's __init__.py. Each re-export list is a strict subset of the package's, so nothing that resolved through a shim can break. Also corrects the path comment on mixins/ddl_table.py, which still named the file before it was renamed from table.py. * docs: rewrite the README and correct the .claude/ knowledge base Removed the __partition__ model attribute from the partitioning example: no such attribute exists, and table partitioning is in fact unsupported by this backend -- add_partition, attach_partition, detach_partition and the three partitioning switches are all False at every probed version. The generated version table is what surfaces that. Corrected the core dependency floor, documented as >=1.0.0 and unresolvable against the 1.0.0.devN pre-release line. In the agent docs: replaced the invented DatabaseCapabilities / _initialize_capabilities / requires_capability system with the actual supports_* protocol switches, and corrected the PYTHONPATH guidance, which omitted tests. * feat(sqlserver): support client certificates on the ODBC connection string encrypt and trust_server_certificate only cover server-side encryption, so a client certificate could not be presented at all. Add client_certificate, client_key, client_key_password and host_name_in_certificate to SQLServerConnectionConfig and emit them as the ClientCertificate/ClientKey/ClientKeyPassword/HostNameInCertificate ODBC keywords from build_connection_string(), with the same four names accepted as backend kwargs. validate_config() now rejects a client_key given without a client_certificate, so the mismatch surfaces as a clear ValueError instead of an opaque driver failure at connect time. * chore: rename changelog fragment to PR #6 * ci: unify the changelog fragment rules Replace the six divergent changelog-check variants with one implementation that encodes the documented merge rules. feature/*|fix/* -> release/*|maint/*|main: - the PR must add at least one changelog.d/<number>.<type>.md - every added fragment's <number> must equal the PR number - fragments already present from previously merged branches are ignored, since only files ADDED by this PR are considered - escape hatches ([trivial] title, skip-changelog label, docs: update CHANGELOG) waive only the "at least one" requirement; the number-must-match rule is always enforced on fragments that were added release/* -> main: - CHANGELOG.md must have changed (content is not inspected) - changelog.d/ must hold no fragments at the merge commit - no escape hatches apply to this path The number-must-match rule was the one documented requirement that none of the previous variants implemented. Two latent bugs are also fixed: the paths-ignore of "**.md" meant a CHANGELOG.md-only push never ran the check at all, and three backends skipped release merges outright instead of applying the release rules. * ci: reject feature/fix branches that target main directly A feature/fix branch merges into a release or maintenance branch; only release/* merges into main. Opening a second PR at main from the same branch is a duplicate, and it also breaks the changelog fragment, which is numbered for exactly one PR. This pattern already cost a wasted PR in six repositories (a ->release PR merged alongside a ->main PR that was then closed), and the fragment numbering check only exposed it indirectly, by failing the leftover PR. hotfix/* -> main stays allowed; the version control policy calls for hotfixes to be merged back to main and the active release branch.
1 parent 61ab594 commit 6fe9527

67 files changed

Lines changed: 637 additions & 623 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.claude/backend_development.md‎

Lines changed: 19 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ rhosocial-activerecord-{backend}/
3232
│ └── impl/
3333
│ └── {backend}/
3434
│ ├── __init__.py
35-
│ ├── backend.py # Main backend implementation
35+
│ ├── backend/ # sync + async backend classes
3636
│ ├── adapters.py # Backend-specific type adapters
3737
│ ├── config.py # Connection configuration
3838
│ ├── dialect.py # SQL dialect handling
@@ -114,7 +114,7 @@ Expression.to_sql() -> Dialect.format_*() -> SQL string and parameters
114114
- `operators.py`: SQL operations (binary, unary, arithmetic expressions)
115115
- `predicates.py`: SQL predicate expressions (WHERE clause conditions)
116116
- `query_parts.py`: SQL query clauses (WHERE, GROUP BY, HAVING, ORDER BY, etc.)
117-
- `statements.py`: DML/DQL/DDL statements (SELECT, INSERT, UPDATE, DELETE, etc.)
117+
- `statements/`: DML/DQL/DDL statements (a package, not a module) (SELECT, INSERT, UPDATE, DELETE, etc.)
118118
- `functions.py`: Standalone factory functions for creating SQL expressions
119119
- `aggregates.py`: SQL aggregation expressions and functions
120120
- `advanced_functions.py`: Advanced SQL functions (CASE, CAST, EXISTS, window functions)
@@ -149,8 +149,9 @@ Every backend must implement the `StorageBackend` abstract base class. This is t
149149
```python
150150
# backend.py
151151
from typing import Any, Dict, List, Optional, Tuple, Type
152-
from ...base import StorageBackend, ConnectionConfig
153-
from ...type_adapter import SQLTypeAdapter
152+
from rhosocial.activerecord.backend.base import StorageBackend
153+
from rhosocial.activerecord.backend.config import ConnectionConfig
154+
from rhosocial.activerecord.backend.type_adapter import SQLTypeAdapter
154155

155156
class MyBackend(StorageBackend):
156157
"""Implementation of MyDatabase backend using native driver."""
@@ -175,7 +176,8 @@ class MyBackend(StorageBackend):
175176
"""Check if connection is valid."""
176177
pass
177178

178-
def execute(self, sql: str, params: Optional[Tuple] = None, returning: Optional[Union[bool, List[str], ReturningOptions]] = None, column_adapters: Optional[Dict[str, Tuple[SQLTypeAdapter, Type]]] = None) -> QueryResult:
179+
def execute(self, sql: str, params: Optional[Tuple] = None, *,
180+
options: Optional[ExecutionOptions] = None) -> QueryResult:
179181
"""Executes a SQL query."""
180182
pass
181183

@@ -197,10 +199,6 @@ class MyBackend(StorageBackend):
197199
"""
198200
pass
199201

200-
def _initialize_capabilities(self) -> DatabaseCapabilities:
201-
"""Declare all features supported by this backend."""
202-
pass
203-
204202
def _handle_error(self, error: Exception) -> None:
205203
"""Catch driver-specific exceptions and raise standard errors."""
206204
pass
@@ -241,10 +239,10 @@ Define an immutable configuration class for your backend.
241239
# config.py
242240
from dataclasses import dataclass, field
243241
from typing import Optional, Dict, Any
244-
from rhosocial.activerecord.backend.config import BaseConfig
242+
from rhosocial.activerecord.backend.config import ConnectionConfig
245243

246-
@dataclass(frozen=True)
247-
class MyDatabaseConfig(BaseConfig):
244+
@dataclass
245+
class MyDatabaseConfig(ConnectionConfig):
248246
host: str = "localhost"
249247
port: int = 5432
250248
database: str
@@ -366,34 +364,20 @@ Every dialect inherits from:
366364

367365
```python
368366
# dialect.py
369-
from rhosocial.activerecord.backend.dialect import (
370-
SQLDialect, ReturningClauseHandler, TypeMapping
371-
)
372-
from typing import Dict, List, Optional
367+
from rhosocial.activerecord.backend.dialect.base import SQLDialectBase
368+
from typing import Optional, Tuple
373369

374-
class MyDatabaseDialect(SQLDialect):
370+
class MyDatabaseDialect(SQLDialectBase):
375371
def __init__(self):
376372
super().__init__()
377373
self.returning_handler = MyDatabaseReturningHandler()
378374

379-
def quote_identifier(self, identifier: str) -> str: return f'"{identifier}"'
380-
def get_placeholder(self, name: str = None) -> str: return "?"
381-
382-
def format_limit_offset(self, sql: str, limit: Optional[int], offset: Optional[int]) -> str:
383-
if limit is not None: sql += f" LIMIT {limit}"
384-
if offset is not None: sql += f" OFFSET {offset}"
385-
return sql
386-
387-
def get_type_mappings(self) -> Dict[str, TypeMapping]:
388-
return {"INTEGER": TypeMapping("INTEGER"), "TEXT": TypeMapping("TEXT"), ...}
375+
def get_parameter_placeholder(self, position: int = 0) -> str:
376+
return "?"
389377

390-
class MyDatabaseReturningHandler(ReturningClauseHandler):
391-
@property
392-
def is_supported(self) -> bool: return True
393-
394-
def format_clause(self, columns: Optional[List[str]] = None) -> str:
395-
cols = "*" if not columns else ', '.join(self.dialect.quote_identifier(c) for c in columns)
396-
return f"RETURNING {cols}"
378+
# Identifier quoting is inherited from SQLDialectBase.format_identifier;
379+
# LIMIT/OFFSET come from the DQL mixins; type mapping is not a dialect
380+
# method but a set of SQLTypeAdapter registrations on the backend.
397381
```
398382

399383
### 4. The Type Adaptation System
@@ -565,7 +549,7 @@ This checklist summarizes all the required and recommended steps for creating a
565549
- [ ] Implement `get_default_adapter_suggestions` to define the conversion strategy.
566550
- [ ] Implement `Transaction Management`.
567551
- [ ] Implement `Error Handling` to map driver exceptions.
568-
- [ ] Implement `Feature Detection` to declare capabilities.
552+
- [ ] Implement the `supports_*` capability switches your dialect needs.
569553
- [ ] If providing an `AsyncStorageBackend`, ensure it has **Functional Equivalence** with the sync version.
570554

571555
### Required Tests

‎.claude/code_style.md‎

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ from pydantic import BaseModel, Field
5656

5757
# Local application imports
5858
from rhosocial.activerecord.model import ActiveRecord
59-
from rhosocial.activerecord.backend import StorageBackend
59+
from rhosocial.activerecord.backend.base import StorageBackend
6060

6161
# Relative imports (within same package)
6262
from .base import BaseActiveRecord
@@ -65,7 +65,7 @@ from ..interface import IActiveRecord
6565

6666
### Line Length and Wrapping
6767

68-
- Maximum line length: 100 characters
68+
- Maximum line length: 120 characters (ruff `line-length = 120`; black formats to 100, but 100 is not the limit)
6969
- Wrap long lines using parentheses:
7070

7171
```python
@@ -291,7 +291,7 @@ from typing import annotations
291291

292292
import third_party
293293

294-
from rhosocial.activerecord import local
294+
from rhosocial.activerecord.model import ActiveRecord
295295
from . import relative
296296

297297
# Constants
@@ -414,12 +414,12 @@ raise DatabaseError(
414414
"""Test module for specific feature."""
415415

416416
import pytest
417-
from rhosocial.activerecord import Model
417+
from rhosocial.activerecord.model import ActiveRecord
418418

419419
# Fixtures at top
420420
@pytest.fixture
421421
def sample_model():
422-
return Model(name="test")
422+
return ActiveRecord(name="test")
423423

424424
# Test classes for organization
425425
class TestCRUDOperations:
@@ -603,13 +603,12 @@ def transaction(backend: StorageBackend):
603603
### Mixin Classes
604604

605605
```python
606-
class TimestampMixin:
607-
"""Add timestamp fields to models."""
608-
created_at: datetime = Field(default_factory=datetime.now)
609-
updated_at: Optional[datetime] = None
610-
611-
def before_save(self):
612-
self.updated_at = datetime.now()
606+
# TimestampMixin declares NO fields: it hooks
607+
# ModelEvent.BEFORE_INSERT / BEFORE_UPDATE and requires the model to
608+
# declare the fields itself, named per __created_at_field__ /
609+
# __updated_at_field__. Use DefaultTimestampMixin to get them declared
610+
# for you. There is no before_save() hook -- lifecycle is registered
611+
# with instance.on(ModelEvent.X, handler).
613612
```
614613

615614
## Expression-Dialect System Coding Standards
@@ -687,7 +686,7 @@ class MyExpression(mixins.ArithmeticMixin, mixins.ComparisonMixin, bases.SQLValu
687686

688687
def to_sql(self) -> 'bases.SQLQueryAndParams':
689688
# Delegate formatting to dialect
690-
formatted_value = self.dialect.format_string_literal(self.value)
689+
formatted_value = self.dialect.format_literal(self.value)
691690
return formatted_value, (self.value,)
692691
```
693692

@@ -705,7 +704,7 @@ def format_identifier(self, identifier: str) -> str:
705704
- [ ] Path comment at file start matches actual location
706705
- [ ] All functions/classes have docstrings
707706
- [ ] Type hints on all function signatures
708-
- [ ] No line exceeds 100 characters
707+
- [ ] No line exceeds 120 characters
709708
- [ ] Imports sorted and grouped correctly
710709
- [ ] No commented-out code
711710
- [ ] Exception messages are informative

‎.claude/commands/gen-model.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ CREATE TABLE users (
5858
2. Show the SQL migration to create the table
5959
3. Remind the user to configure the backend:
6060
```python
61-
from rhosocial.activerecord.backend.impl.sqlite import SQLiteBackend
61+
from rhosocial.activerecord.backend.impl.sqlite.backend import SQLiteBackend
6262
User.configure(SQLiteBackend("sqlite:///app.db"))
6363
```
6464

‎.claude/commands/validate-model.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ user = User(name="John") # Will raise "No backend configured"
6666
user.save()
6767

6868
# CORRECT
69-
from rhosocial.activerecord.backend.impl.sqlite import SQLiteBackend
69+
from rhosocial.activerecord.backend.impl.sqlite.backend import SQLiteBackend
7070
User.configure(SQLiteBackend("sqlite:///app.db"))
7171
user = User(name="John")
7272
user.save()

‎.claude/testing.md‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ dependencies = [
2323

2424
```bash
2525
cd /mnt/i/GitHubRepositories/rhosocial/python-activerecord-sqlserver
26-
source .venv/bin/activate
27-
export PYTHONPATH=src
26+
source .venv3.14-ubuntu26.04/bin/activate
27+
export PYTHONPATH=src:tests
2828
pytest tests/ -v
2929
```
3030

@@ -36,5 +36,5 @@ pytest tests/ -v
3636

3737
## Reference
3838

39-
- [Core testing guide](../python-activerecord/.claude/testing.md)
40-
- [SQL Server backend development](../python-activerecord/.claude/backend_development.md)
39+
- [Core testing guide](../../python-activerecord/.claude/testing.md)
40+
- [SQL Server backend development](../../../python-activerecord/.claude/backend_development.md)

0 commit comments

Comments
 (0)