feat(records): filter endpoint#2699
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2699 +/- ##
========================================
Coverage 93.67% 93.67%
========================================
Files 503 503
Lines 50696 50840 +144
========================================
+ Hits 47488 47624 +136
- Misses 3208 3216 +8
🚀 New features to boost your workflow:
|
andersfylling
force-pushed
the
andersf/records/list5654
branch
from
664dc22 to
299928a
Compare
| ) | ||
| self._files = FileConcurrencyConfig(self, read=4, write=2, upload=5, download=5, delete=2, open_files=15) | ||
| self._records = RecordsGlobalConcurrencyConfig(self, write=20) | ||
| self._records = RecordsGlobalConcurrencyConfig(self, read=4, write=20) |
There was a problem hiding this comment.
4 is the lowest bucket used across all query endpoints https://api-docs.cognite.com/20230101/tag/Records#section/Rate-and-concurrency-limits
andersfylling
force-pushed
the
andersf/records/list5654
branch
from
299928a to
b5cec33
Compare
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements the list (filter) endpoint for stream records, introducing the Record, RecordList, TimeRange, and RecordSourceSelector data classes, read concurrency support, and corresponding unit tests. The review feedback correctly identifies three key issues: a breaking change in the RecordsGlobalConcurrencyConfig constructor signature, a potential crash if the typing field in the API response is null, and an AttributeError when dictionary-based sort parameters are passed. Additionally, the newly added unit tests for load and dump methods on the data classes violate the repository style guide, as these are tested automatically.
| def __init__( | ||
| self, | ||
| concurrency_settings: ConcurrencySettings, | ||
| read: int, | ||
| write: int, | ||
| ) -> None: | ||
| super().__init__(concurrency_settings, "records", read=0, write=write, delete=0) | ||
| super().__init__(concurrency_settings, "records", read=read, write=write, delete=0) |
There was a problem hiding this comment.
Changing the constructor signature of RecordsGlobalConcurrencyConfig by inserting read before write is a breaking change for any external code or subclasses that instantiate this class using positional arguments. To maintain backward compatibility, keep write as the second positional argument and make read an optional keyword-only or defaulted argument placed after write.
def __init__(\n self,\n concurrency_settings: ConcurrencySettings,\n write: int,\n read: int = 4,\n ) -> None:\n super().__init__(concurrency_settings, \"records\", read=read, write=write, delete=0)|
|
||
| @classmethod | ||
| def _load_raw_api_response(cls, responses: list[dict[str, Any]]) -> Self: | ||
| typing = next((TypeInformation._load(resp["typing"]) for resp in responses if "typing" in resp), None) |
There was a problem hiding this comment.
If the API response contains a "typing" key but its value is null (None), resp["typing"] will be None, causing TypeInformation._load(None) to raise a TypeError or KeyError. Guard against this by checking if resp.get("typing") is not None.
typing = next((TypeInformation._load(resp[\"typing\"]) for resp in responses if resp.get(\"typing\") is not None), None)| if sort is not None: | ||
| sort_list = [sort] if isinstance(sort, InstanceSort) else list(sort) | ||
| other_params["sort"] = [spec.dump() for spec in sort_list] |
There was a problem hiding this comment.
If a user passes a dictionary or a list of dictionaries for the sort parameter (which is a common pattern in the SDK), isinstance(sort, InstanceSort) will evaluate to False, and list(sort) on a dictionary will return its keys. This leads to an AttributeError when trying to call .dump() on a string key. Update the logic to support both InstanceSort and dictionary representations gracefully.
if sort is not None:\n sort_list = [sort] if isinstance(sort, (InstanceSort, dict)) else list(sort)\n other_params[\"sort\"] = [spec.dump() if isinstance(spec, InstanceSort) else spec for spec in sort_list]
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 participant
Adds the filter records endpoint to the SDK.
documentation: https://api-docs.cognite.com/20230101/tag/Records/operation/filterRecords
Note
Original PR here: #2680
@haakonvt made a few comments that are still relevant:
review comment: We need to support pagination, it is one of the key features of the SDK 😄response: I want to push back on this, and say we shouldn't offer pagination to begin with. At least not until the api has implemented it. There is also the sync endpoint where users can a large volume of records (feat(records): sync endpoint #2698).