Skip to content
This repository has been archived by the owner on Jul 18, 2023. It is now read-only.

Directory storage engine; Switch to cryptonite base conversion functions #38

Merged
merged 4 commits into from
Jun 10, 2017

Conversation

patrickmn
Copy link
Contributor

@patrickmn patrickmn commented Jun 7, 2017

  • Adds a dir storage engine, specificable with --storage=dir:path. Constellation will create a file for each payload in path/payloads. Important to note is that payload filenames will be Base32-encoded to ensure compatibility with case-insensitive file systems. (The API uses Base64 everywhere else.)

    This is intended to be useful for any storage engine that has a FUSE connector. Please note that if the file system is distributed, a payload must be readable by other nodes immediately after it has been written (at least if used with Quorum.)

  • Changes wording for storage backend error messages when a payload isn't found from "key not found" to "payload not found"

  • Simplifies some of the ByteArray Base64 conversion stuff.

-- [] -> Left "load: No payload found"
-- _ -> Left "load: More than one payload found"
-- TODO: In testStorage, don't rely on the presence of the strings below
[] -> Left "Key not found in SQLite"
Copy link

Choose a reason for hiding this comment

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

@patrickmn actually I was thinking of changing the error text for all of the storage types to "payload not found" as you originally had here, since users seem to be confusing "key" with "private/public key".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@patrickmn patrickmn merged commit cac3e77 into master Jun 10, 2017
@patrickmn patrickmn deleted the directory-storage branch June 10, 2017 02:52
Copy link
Contributor

@conor10 conor10 left a comment

Choose a reason for hiding this comment

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

👍 The directory storage engine is a nice addition

storage <- case break (== ':') cfgStorage of
("bdb", ':':s) -> berkeleyDbStorage s
("dir", ':':s) -> directoryStorage s
_ -> berkeleyDbStorage cfgStorage -- Default
-- storage <- memoryStorage
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason not to include this as an option for users with the new command line options?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Command line options are parsed to create the cfg, so --storage "dir:..." would trigger this

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I was referring to the memoryStorage option, not directoryStorage

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have another PR coming which will enable that and others, just trying to decide if making leveldb a base dependency is worth it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's worth having for now, until a final decision is made wrt storage to use - that way you don't have to have the commented out tests and dependencies in the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah was leaning the same way. Will open another PR soon, including some benchmarking code as well...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants