Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Update Transaction trait and implementations to return counts #199

Closed
wants to merge 2 commits into from
Closed

[WIP] Update Transaction trait and implementations to return counts #199

wants to merge 2 commits into from

Conversation

ozgrakkurt
Copy link
Contributor

Update Transaction trait and implementations to return counts from set and delete operations for checking if operation succeeded.

closes #192

update Transaction trait so it returns counts from set and delete operations.
Update MemoryDatastore so it returns counts from set and delete operations.
@ozgrakkurt
Copy link
Contributor Author

@ysimonson Can you review the changes I made to MemoryDatastore when you have time?


let mut deletable_vertex_properties: Vec<(Uuid, String)> = Vec::new();
let mut deletable_vertex_properties: Vec<(Uuid, String)> = Vec::new();
Copy link
Member

Choose a reason for hiding this comment

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

This isn't used at all

}

let mut deletable_edges: Vec<EdgeKey> = Vec::new();
self.vertex_properties.remove(&property_key);
Copy link
Member

Choose a reason for hiding this comment

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

This is iterating through self.vertex_properties and removing elements at the same time, which should fail AFAIK

@ysimonson
Copy link
Member

ysimonson commented Nov 13, 2021

You can test out these changes by cding into lib and running cargo test --features=test-suite. That will also disregard errors in rocksdb and sled not having implemented the trait changes yet.

@ysimonson
Copy link
Member

@ozgrakkurt how's progress on this? I'm wondering if I should wait for this before releasing 3.0.

@ozgrakkurt
Copy link
Contributor Author

@ozgrakkurt how's progress on this? I'm wondering if I should wait for this before releasing 3.0.

I hope I can finish it by the end of the week.

@ysimonson
Copy link
Member

I'm going to close this out since there hasn't been progress in the past few months. Hoping you can circle back to this at some point though @ozgrakkurt!

@ysimonson ysimonson closed this Aug 10, 2022
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.

Calling set_edge_properties doesn't return error if edge doesn't exist
2 participants