Skip to content

fix(records): raise TypeError instead of leaking AttributeError for typed arguments - #2749

Draft
andersfylling wants to merge 3 commits into
masterfrom
fix/records-typed-request-arguments
Draft

fix(records): raise TypeError instead of leaking AttributeError for typed arguments#2749
andersfylling wants to merge 3 commits into
masterfrom
fix/records-typed-request-arguments

Conversation

@andersfylling

Copy link
Copy Markdown
Contributor

Reported in review: passing a raw dict where a typed request object is expected fails deep inside the SDK, naming neither the argument nor the type.

client.data_modeling.records.filter(
    stream_id=STREAM_ID,
    last_updated_time={"gte": since_str},   # dict instead of TimeRange
    filter=HasData(containers=[(SPACE, CONTAINER)]),
)
# AttributeError: 'dict' object has no attribute 'dump'

client.data_modeling.records.filter(stream_id=STREAM_ID, sort=["lastUpdatedTime"])
# AttributeError: 'str' object has no attribute 'dump'

last_updated_time, sort, sources, target_units and filter now share one dump helper: the typed object is dumped, a dict is passed through as the wire form it already is, and anything else raises TypeError naming the argument, the expected type and a worked example. sources and target_units additionally accept a single object instead of requiring a one-item sequence — target_units=RecordTargetUnit(...) used to fail with 'RecordTargetUnit' object is not iterable.

Accepting dicts is what makes the reported snippet work rather than merely fail better, and it matches instances.list, which already takes InstanceSort | dict. The signatures are widened to match, so a dict is no longer a type-checker error at the call site either.

First commit is the failing tests, second is the fix, including the regenerated sync client.

🤖 Generated with Claude Code

andersfylling and others added 3 commits August 7, 2026 01:00
last_updated_time, sort, sources, target_units and filter are dumped without a
type check, so a raw dict or a bare string surfaces as
"AttributeError: dict object has no attribute dump" from deep inside the SDK,
naming neither the argument nor the expected type. The dict cases are the wire
form the endpoints already accept for filter, so they should just work.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…yped arguments

last_updated_time, sort, sources, target_units and filter now go through one
dump helper: the typed object is dumped, a dict is passed through as the wire
form it already is, and anything else raises TypeError naming the argument, the
expected type and a worked example. sources and target_units also accept a
single object instead of requiring a one-item sequence.

The signatures are widened accordingly, so a dict is no longer a type error at
the call site either, matching how instances.list already takes InstanceSort | dict.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Aug 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.46835% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.79%. Comparing base (629c506) to head (58d988e).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
cognite/client/_api/data_modeling/records.py 94.28% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2749   +/-   ##
=======================================
  Coverage   93.79%   93.79%           
=======================================
  Files         513      513           
  Lines       52445    52515   +70     
=======================================
+ Hits        49189    49256   +67     
- Misses       3256     3259    +3     
Files with missing lines Coverage Δ
cognite/client/_sync_api/data_modeling/records.py 100.00% <100.00%> (ø)
...s_unit/test_api/test_data_modeling/test_records.py 100.00% <100.00%> (ø)
cognite/client/_api/data_modeling/records.py 98.13% <94.28%> (+0.59%) ⬆️

... and 6 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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