Remove Link #780
Remove Link #780
Conversation
cas_implementations/src/eav/file.rs
Outdated
| @@ -90,6 +90,24 @@ impl EavFileStorage { | |||
| Ok(()) | |||
| } | |||
|
|
|||
| fn delete_file( | |||
There was a problem hiding this comment.
Am I incorrect in thinking that the EAV was append only?
@lucksus
There was a problem hiding this comment.
We already synced in heartbeat and currently implementing eavindexing before this can get in
There was a problem hiding this comment.
ah, I see. Sorry to interrupt!
Do you mean that you are currently switched to implementing indexing the EAV before proceeding further with this PR?
There was a problem hiding this comment.
No problem! Yes :) Deleting will simply be changing the hash to a new one which does not contain the data! Working on that right now
|
@AshantiMutinta for my documentation work, I'm trying to make sense of |
core_types/src/eav.rs
Outdated
| .unwrap_or_else(|| { | ||
| let latest = get_latest(e.clone(), map.clone()) | ||
| let latest = get_latest(e.clone(), map.clone(), index_query.clone()) | ||
| .unwrap_or(EntityAttributeValueIndex::default()); |
There was a problem hiding this comment.
You can just write unwrap_or_default()
core/src/network/handler/store.rs
Outdated
| ) | ||
| .expect("dht_meta_data should be EntryWithHader"); | ||
| thread::spawn(move || { | ||
| match context.block_on(remove_link_workflow(&entry_with_header, &context.clone())) { |
There was a problem hiding this comment.
You could make this much cleaner by using an if let expression
There was a problem hiding this comment.
Agreed! I think I found it structured like this and just added to it. In imo it would be a good candidate for a refactor since we are trying to get this story in for the closed alpha
There was a problem hiding this comment.
Then again it's not so hard to refactor a little as we go when we see easy improvements that would take almost no time...
There was a problem hiding this comment.
You are right, let me change that
| Ok(filtered | ||
| .into_iter() | ||
| .filter(|eav| eav.attribute().starts_with("link__")) | ||
| .collect()) |
There was a problem hiding this comment.
BTW, we clarified this per voice-chat yesterday. So fetch_eavi only returns the latest version of potentially multiple EAVs that match the query. With the prefixes set in line 61, all EAVs where the attribute matches either link__<tag> or removed_link_<tag> are considered the same. If we get the latest of these each, it will be either a link__<tag> or removed_link_<tag>, depending on if the last action (for this specific link!) was an add or removal. So yes, if remove all those links for which the latest action is a removal we get exactly what we want.
And, this could get more documented and tested :)
Co-Authored-By: AshantiMutinta <AshantiMutinta@gmail.com>
core/src/dht/dht_reducers.rs
Outdated
| new_store.actions_mut().insert( | ||
| action_wrapper.clone(), | ||
| Err(HolochainError::ErrorGeneric(String::from( | ||
| "Base for link not found for remov", |
Hello! I think I am having some trouble with cargo fmt issues with it affecting my line endings. I will use notepad to remove them, also thanks, typo has been fixed |
This pull request makes it able to remove link from local
The files changed are this number because my cargo fmt adds windows line endings, so error on my part. Should be harmless!