Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Remove invalid expectation (beta) #4542

Merged
merged 1 commit into from
Feb 15, 2017
Merged

Remove invalid expectation (beta) #4542

merged 1 commit into from
Feb 15, 2017

Conversation

NikVolf
Copy link
Contributor

@NikVolf NikVolf commented Feb 14, 2017

#4420

code looks like a mess there, and invalid expect is like an icy on the cake

@NikVolf NikVolf added A0-pleasereview 🤓 Pull request needs code review. A8-backport 🕸 Pull request is already reviewed well in another branch. M4-core ⛓ Core client code / Rust. labels Feb 14, 2017
@rphmeier rphmeier added B0-patch A8-looksgood 🦄 Pull request is reviewed well. and removed A8-backport 🕸 Pull request is already reviewed well in another branch. A0-pleasereview 🤓 Pull request needs code review. labels Feb 14, 2017
@rphmeier
Copy link
Contributor

Build of rocksdb-sys failing with the gcc on the CI? @General-Beck

@General-Beck
Copy link
Contributor

@rphmeier no, build succeful

@tomusdrw
Copy link
Collaborator

Can you elaborate why the expectation was incorrect? Seems that fn get always reads the account from cache, no?
So the only reason I see is that there is a race condition when trying to remove the same account twice, was there any other issue?

@NikVolf
Copy link
Contributor Author

NikVolf commented Feb 15, 2017

@tomusdrw
exactly this
with concurrent access the expectation can't be 100% correct, because there is no single lock

and ui/user might duplicate delete account request so that it results in a panic

@rphmeier
Copy link
Contributor

@General-Beck this one looks pretty failed to me: https://gitlab.ethcore.io/parity/parity/builds/46155

@arkpar
Copy link
Collaborator

arkpar commented Feb 15, 2017

I thought we don't support armv6. Where's the PR check for that target coming from?

@gavofyork gavofyork added A7-looksgoodtestsfail 🤖 Pull request is reviewed well, but cannot be merged due to tests failing. and removed A8-looksgood 🦄 Pull request is reviewed well. labels Feb 15, 2017
@arkpar arkpar added A8-backport 🕸 Pull request is already reviewed well in another branch. and removed B0-patch labels Feb 15, 2017
@NikVolf
Copy link
Contributor Author

NikVolf commented Feb 15, 2017

test fail is unrelated to the pr

@arkpar arkpar added A8-looksgood 🦄 Pull request is reviewed well. and removed A7-looksgoodtestsfail 🤖 Pull request is reviewed well, but cannot be merged due to tests failing. labels Feb 15, 2017
@arkpar arkpar merged commit f4f7b83 into beta Feb 15, 2017
@arkpar arkpar deleted the fix-beta-expect branch February 15, 2017 18:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-backport 🕸 Pull request is already reviewed well in another branch. A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants