-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Nullable in Storage classes #3670
Conversation
src/Neo/Persistence/ClonedCache.cs
Outdated
} | ||
|
||
protected override void UpdateInternal(StorageKey key, StorageItem value) | ||
{ | ||
innerCache.GetAndChange(key).FromReplica(value); | ||
_innerCache.GetAndChange(key)?.FromReplica(value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fail before, is this expected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fail before, is this expected?
I think it should be a bug.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand why SnapshotCache
read from store, but write if the snapshot
exists...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because I want to avoid logic changes in this PR, I will throw a different exception, taking a look to the code it seems that it should always exists or it's a failure
Co-authored-by: Christopher Schuchardt <[email protected]>
Co-authored-by: Christopher Schuchardt <[email protected]>
Co-authored-by: Christopher Schuchardt <[email protected]>
…into storage-nullable
Co-authored-by: Christopher Schuchardt <[email protected]>
Co-authored-by: nan01ab <[email protected]>
…into storage-nullable
@neo-project/core could you take a look to this one please |
@@ -239,18 +233,18 @@ public void Delete(StorageKey key) | |||
} | |||
if (seek_prefix == null) | |||
{ | |||
throw new ArgumentException(); | |||
throw new ArgumentException($"{nameof(key_prefix)} with all bytes being 0xff is not supported now"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is all 0xff
not support? Then SeekDirection
should be only Forward
and removed from the parameters of this function.
You need 0xff
to go Backward
for SeekDirection
. ApplicationLogs
uses this method. However I used IStore
interface to do what this can't do. If you can explain whats the problem. I can fix it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is all
0xff
not support? ThenSeekDirection
should be onlyForward
and removed from the parameters of this function.You need
0xff
to goBackward
forSeekDirection
.ApplicationLogs
uses this method. However I usedIStore
interface to do what this can't do. If you can explain whats the problem. I can fix it.
I have a fix, waiting for this PR to merge.
@nan01ab PR has been merge please add your PR for the |
Description
Required for #3669
These changes are required for #3669, was found some errors with nullables, and it require to be solved first
Fixes # (issue)
Type of change
How Has This Been Tested?
Test Configuration:
Checklist: