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

fixed #1889, .DS_Store is no longer treated as key file #1892

Merged
merged 5 commits into from
Aug 10, 2016

Conversation

debris
Copy link
Collaborator

@debris debris commented Aug 9, 2016

No description provided.

@debris debris added A0-pleasereview 🤓 Pull request needs code review. A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. and removed A0-pleasereview 🤓 Pull request needs code review. A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Aug 9, 2016
@arkpar
Copy link
Collaborator

arkpar commented Aug 9, 2016

Wouldn't it be better to only open .json files?

@debris
Copy link
Collaborator Author

debris commented Aug 9, 2016

key file name does not contain .json suffix

@arkpar
Copy link
Collaborator

arkpar commented Aug 9, 2016

Or ignore hidden files at least. It's the same story with thumbs.db on windows

@tomusdrw
Copy link
Collaborator

tomusdrw commented Aug 9, 2016

Why do we panic instead of just displaying a warning that key file couldn't have been loaded?

@debris
Copy link
Collaborator Author

debris commented Aug 9, 2016

Why do we panic instead of just displaying a warning that key file couldn't have been loaded?

Technically it's not a panic, but quick exit. We assume that our keys must be valid. We display warnings only if geth keys cannot be imported.

@coveralls
Copy link

coveralls commented Aug 9, 2016

Coverage Status

Changes Unknown when pulling bf79187 on fixed_dsstore into * on master*.

@NikVolf NikVolf added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Aug 9, 2016
@gavofyork
Copy link
Contributor

gavofyork commented Aug 9, 2016

I'm in favour of displaying a warning, rather than outright bailing, when a file in our keys directory cannot be properly loaded as a key.

Bailing should be reserved for unrecoverable errors rather than uncertainty over whether the user actually intended a particular file to be loaded as a key.

@arkpar arkpar added A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. and removed A8-looksgood 🦄 Pull request is reviewed well. labels Aug 9, 2016
@arkpar
Copy link
Collaborator

arkpar commented Aug 9, 2016

does not build

@coveralls
Copy link

coveralls commented Aug 10, 2016

Coverage Status

Coverage increased (+0.006%) to 86.347% when pulling 48059a7 on fixed_dsstore into 4efddb9 on master.

@debris debris added A0-pleasereview 🤓 Pull request needs code review. and removed A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. labels Aug 10, 2016
@NikVolf NikVolf added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Aug 10, 2016
@gavofyork gavofyork merged commit 464516d into master Aug 10, 2016
@gavofyork gavofyork deleted the fixed_dsstore branch August 10, 2016 14:42
arkpar pushed a commit that referenced this pull request Aug 12, 2016
* fixed #1889, .DS_Store is no longer treated as key file

* ethstore filters directories, hidden files and common system files

* fixed compiling
arkpar added a commit that referenced this pull request Aug 12, 2016
* RPC errors & logs (#1845)

* Refactoring errors in RPC

* Updating jsonrpc-core

* Fixing code_at

* Avoid mentioning obvious segments in proof

[ci:skip]

* fixed cache_manager lock order

* Purging .derefs, fixing clippy warnings. (#1890)

* Fixing clippy warnings

* Purging derefs

* Simplifying engine derefs

* Simplifying more engine derefs

* Adding more details to miner log

* fixed #1889, .DS_Store is no longer treated as key file (#1892)

* fixed #1889, .DS_Store is no longer treated as key file

* ethstore filters directories, hidden files and common system files

* fixed compiling

* fix regression with geth dir

* fix regression with geth dir

* Fix ipc compilation and add ipc feature to test targets (#1902)

* fix compilation and add it to the ci run

* no separator?

* use quotes and spaces

* RocksDB version bump

* Don't return deleted nodes that are not yet flushed (#1908)

* polling & connection timeouts (#1910)

* Peers RPC + UI displaying active/connected/max peers (#1915)

* Peers API

* Bumping Parity-UI

* Fixing tests

* Save nodes removed from backing_overlay until commit (#1917)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants