Skip to content

refactor!: Align DatasetClient and KeyValueStoreClient interfaces with their Python counterparts#3627

Open
janbuchar wants to merge 2 commits into
v4from
simplify-storage-subclients
Open

refactor!: Align DatasetClient and KeyValueStoreClient interfaces with their Python counterparts#3627
janbuchar wants to merge 2 commits into
v4from
simplify-storage-subclients

Conversation

@janbuchar
Copy link
Copy Markdown
Contributor

@janbuchar janbuchar commented May 5, 2026

@janbuchar janbuchar added the t-tooling Issues with this label are in the ownership of the tooling team. label May 5, 2026
@janbuchar janbuchar changed the title refactor!: Trim the storage subclient interfaces refactor!: Align DatasetClient and KeyValueStoreClient interfaces with their Python counterparts May 7, 2026
@janbuchar janbuchar requested review from B4nan and barjin May 7, 2026 10:39
@janbuchar janbuchar marked this pull request as ready for review May 7, 2026 10:39
Copy link
Copy Markdown
Member

@barjin barjin left a comment

Choose a reason for hiding this comment

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

Thank you @janbuchar ! Few ideas below ⬇️

Comment thread packages/core/src/storages/dataset.ts Outdated
* @ignore
*/
export function checkAndSerialize<T>(item: T, limitBytes: number, index?: number): string {
export function isJsonSerializable<T>(item: T, index?: number): void {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A method called >>is<<JsonSerializable should imo return a Boolean... I certainly wouldn't expect it to throw as a part of the "happy" path.

Comment thread packages/core/src/storages/key_value_store.ts Outdated
Base automatically changed from refactor-storage-manager to v4 May 11, 2026 11:18
@janbuchar janbuchar force-pushed the simplify-storage-subclients branch from d998bc7 to 1e349f5 Compare May 12, 2026 11:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

t-tooling Issues with this label are in the ownership of the tooling team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants