fix: `store_path` should create missing directories, like `load_path` #60

PiDelport · 2022-03-03T12:13:28Z

Currently, load, load_path, and store will create any missing parent directories in the config file path, but store_path won't.

This adds some test coverage, and moves the create_dir_all call from store into store_path, which should fix the issue and align the behaviour of all four functions.

matthiasbeyer

I don't feel qualified enough to either approve or reject this PR as a whole - someone else from the @rust-cli/maintainers should do that.

Still, I would like to see a small addition (as noted), either in this PR (if the author wants to add that piece) or in a seperate PR 👍

matthiasbeyer · 2022-03-03T14:19:53Z

src/lib.rs

 /// [`store`]: fn.store.html
 pub fn store_path<T: Serialize>(path: impl AsRef<Path>, cfg: T) -> Result<(), ConfyError> {
+    let path = path.as_ref();
+    fs::create_dir_all(path.parent().unwrap()).map_err(ConfyError::DirectoryCreationFailed)?;


Mind to add another commit that replaces the unwrap() call with an error? 😄

Cool, I replaced this panic with an error result in 2653308.

Does that resolve this?

PiDelport · 2022-03-28T13:07:17Z

@matthiasbeyer: It doesn't look like I can tag @rust-cli/maintainers for a review. Could you perhaps do that?

Dylan-DPC · 2022-03-28T13:19:07Z

Thanks

PiDelport · 2022-03-28T13:20:43Z

Thanks! 😊

Upstream PR: rust-cli/confy#60 (fix: `store_path` should create missing directories, like `load_path`)

* feat(ntc-vault-cli): initial layout * style(ntc-vault-cli): add rustfmt.toml * feat(ntc-vault-cli): add clap-based CLI skeleton * feat(ntc-vault-cli): flesh out structure, implement identity subcommand * refactor(ntc-vault-cli): separate code into two crates: core, cli * refactor(ntc-vault-cli): move rand-using code from core to cli * test(ntc-vault-cli): generate_secure_seed smoke test * feat(ntc-vault-cli): data package and JSON Schema work-in-progress * refactor: move crates into rust-workspace * docs: add comment about using "cargo +nightly fmt" * docs: add ARCHITECTURE.md * refactor: rename package "ntc-vault-core" → "ntc-data-packages" * feat(ntc-data-packages): replace anyhow with thiserror * build(ntc-data-packages): drop jsonschema's default features We don't need jsonschema's CLI, or the file / HTTP resolving features. This greatly reduces our dependency tree. * build: declare rust-version * docs: add package descriptions * feat(ntc-data-packages): better validation error messages * ci(rust-workspace): add check and test workflows for GitHub Actions * docs: fix rustdoc link issues * build(rust-workspace): add rust-toolchain.toml, with channel = "stable" * docs(README): add link to ARCHITECTURE.md * deps(ntc-vault-cli): update confy revision for store_path fix Upstream PR: rust-cli/confy#60 (fix: `store_path` should create missing directories, like `load_path`) * refactor(ntc-data-packages): move tests to API-based integration tests Motivation: https://matklad.github.io/2021/02/27/delete-cargo-integration-tests.html * refactor(ntc-vault-cli): move try_exists to compat module * refactor(ntc-vault-cli): VaultIdentityConfig: make pub * deps(ntc-vault-cli): add dev dependencies for CLI tests * test(ntc-vault-cli): add snapshot-based CLI tests * deps(ntc-data-packages): add serde * feat(ntc-data-packages): add Metadata::from_json_bytes * feat(ntc-vault-cli): add fs_io, with read_metadata * feat(ntc-vault-cli): implement more of the "data" subcommand * chore: add todos to keep track of changes that needs to be made to allign with design * chore(deps): update dependencies and bump jsonschema to 0.16 Co-authored-by: Herman <herman@ntls.io>

* feat: add Vault CLI & data package library (#1) * feat(ntc-vault-cli): initial layout * style(ntc-vault-cli): add rustfmt.toml * feat(ntc-vault-cli): add clap-based CLI skeleton * feat(ntc-vault-cli): flesh out structure, implement identity subcommand * refactor(ntc-vault-cli): separate code into two crates: core, cli * refactor(ntc-vault-cli): move rand-using code from core to cli * test(ntc-vault-cli): generate_secure_seed smoke test * feat(ntc-vault-cli): data package and JSON Schema work-in-progress * refactor: move crates into rust-workspace * docs: add comment about using "cargo +nightly fmt" * docs: add ARCHITECTURE.md * refactor: rename package "ntc-vault-core" → "ntc-data-packages" * feat(ntc-data-packages): replace anyhow with thiserror * build(ntc-data-packages): drop jsonschema's default features We don't need jsonschema's CLI, or the file / HTTP resolving features. This greatly reduces our dependency tree. * build: declare rust-version * docs: add package descriptions * feat(ntc-data-packages): better validation error messages * ci(rust-workspace): add check and test workflows for GitHub Actions * docs: fix rustdoc link issues * build(rust-workspace): add rust-toolchain.toml, with channel = "stable" * docs(README): add link to ARCHITECTURE.md * deps(ntc-vault-cli): update confy revision for store_path fix Upstream PR: rust-cli/confy#60 (fix: `store_path` should create missing directories, like `load_path`) * refactor(ntc-data-packages): move tests to API-based integration tests Motivation: https://matklad.github.io/2021/02/27/delete-cargo-integration-tests.html * refactor(ntc-vault-cli): move try_exists to compat module * refactor(ntc-vault-cli): VaultIdentityConfig: make pub * deps(ntc-vault-cli): add dev dependencies for CLI tests * test(ntc-vault-cli): add snapshot-based CLI tests * deps(ntc-data-packages): add serde * feat(ntc-data-packages): add Metadata::from_json_bytes * feat(ntc-vault-cli): add fs_io, with read_metadata * feat(ntc-vault-cli): implement more of the "data" subcommand * chore: add todos to keep track of changes that needs to be made to allign with design * chore(deps): update dependencies and bump jsonschema to 0.16 Co-authored-by: Herman <herman@ntls.io> * feat: add initial TEE server (#3) * build(rust-sgx-workspace): add workspace for SGX code * build(ntc-tee-server): add SGX project for the TEE server * build(ntc-tee-server): bump Rust edition: "2018" → "2021" * build(ntc-tee-server): better error message for missing Enclave_u library * ci(rust-sgx-workspace): add check and test workflows for GitHub Actions * ci: use nested checkout for the multiple repositories to prevent _temp being deleted * style: fix clippy warnings * ci: remove --all-targets build arg to fix enclave builds and checks Co-authored-by: Herman <herman@ntls.io> * license: relicense to AGPL-3.0 (#17) * ci: use ntls-io fork of rust-sgx-sdk-env (#39) Co-authored-by: Jean-Pierre de Villiers <jeanpierre@jdvl.io> * feat: create oracle-node API (#18) * feat(ntc-oracle-node): connect to PureStake Indexer using algonaut * feat(ntc-oracle-node): add config crate * feat(ntc-oracle-node): add initial server with axum * feat(ntc-oracle-node): implement get_auth_data * feat(ntc-oracle-node): use ed25519 to sign AuthData with ring-compat * style(ntc-oracle-node): apply rustfmt.toml * refactor(ntc-oracle-node): move auth_data functions into handler * feat(ntc-oracle-node): add anyhow error handling * feat(ntc-oracle-node): add unit test for sign_auth_data * feat(ntc-oracle-node): use serde_with to base64 encode SignedAuthData response * refactor(ntc-oracle-node): use cfg(test) in place of allow(dead_code) * feat: add Vault backend (#61) * feat(web-server): copy web-server from Nautilus Wallet * docs(web-server): fix comments in rust-toolchain and rustfmt toml files * refactor: move http-service-impl and sgx-helpers into rust-sgx-workspace * refactor: move sgx-wallet-impl into rust-sgx-workspace but exclude as member * refactor: move sgx-wallet into rust-sgx-workspace but exclude as member * refactor: move sgx-wallet-test into rust-sgx-workspace but exclude as member * refactor: remove web-server * docs(http-service-impl): fix sgx-wallet-impl broken intra doc link * fix(sgx-wallet): fix path to sgx-wallet-impl in Makefile * refactor(rust-sgx-workspace): rename 'wallet' to 'vault' (#62) * refactor(sgx-vault-impl): change vault id to algorand address (#65) * refactor: remove Onfido operations and XRP related code (#66) * feat(rust-sgx-workspace): add Docker build definition for Vault server (#67) * feat: Create two APIs for data operations * Merge branch 'data-operations' of https://github.com/ntls-io/nautilus-trusted-compute into data-operations * feat(data-operations-api): create new project dir * feat: Add base crate * feat: Add basic Actix server * feat: Configure server + add logs_handlers * feat: Fetching logs * feat: Adding logs * refactor: Update log -> data * feat: Add filter functionality * refactor: remove unused imports + fix snake_case * feat(backend-services): initial commit for api * feat(backend-services): initial commit for api * fix(backend-services): add env to .gitignore. * fix (backend-services): .gitignore and datetime * feature (backend-services): add example json and schema files --------- Co-authored-by: Pi Delport <pi@ntls.io> Co-authored-by: Herman <herman@ntls.io> Co-authored-by: Jean-Pierre de Villiers <jeanpierre@jeanpierredevilliers.xyz> Co-authored-by: Jean-Pierre de Villiers <jeanpierre@jdvl.io> Co-authored-by: Bill Guo <billguo99@gmail.com> Co-authored-by: Bill Guo <bill@ntls.io> Co-authored-by: binglekruger <bingkruger@hotmail.co.za>

PiDelport mentioned this pull request Mar 3, 2022

feat: add Vault CLI & data package library ntls-io/nautilus-trusted-compute#1

Merged

matthiasbeyer requested changes Mar 3, 2022

View reviewed changes

PiDelport added 3 commits March 20, 2022 17:29

test: rename test_store_path → test_store_path_atomic

89822e6

test: add tests for load_path and store_path

a6dc791

fix: store_path should create missing directories, like load_path

d0e9a53

PiDelport force-pushed the fix-store-path-missing-directory branch from ebd6720 to d0e9a53 Compare March 20, 2022 15:30

PiDelport added 3 commits March 20, 2022 17:53

refactor: use Option::ok_or

3b58ab3

refactor!: parameterise ConfyError::BadConfigDirectory with message

1713f83

feat: store_path: replace Path::parent panic with BadConfigDirectory

2653308

Dylan-DPC merged commit 6428224 into rust-cli:master Mar 28, 2022

PiDelport added a commit to ntls-io/nautilus-trusted-compute that referenced this pull request Mar 28, 2022

deps(ntc-vault-cli): update confy revision for store_path fix

bc26c28

Upstream PR: rust-cli/confy#60 (fix: `store_path` should create missing directories, like `load_path`)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: `store_path` should create missing directories, like `load_path` #60

fix: `store_path` should create missing directories, like `load_path` #60

Uh oh!

PiDelport commented Mar 3, 2022

Uh oh!

matthiasbeyer left a comment

Uh oh!

matthiasbeyer Mar 3, 2022

Uh oh!

PiDelport Mar 20, 2022 •

edited

Loading

Uh oh!

PiDelport commented Mar 28, 2022

Uh oh!

Dylan-DPC commented Mar 28, 2022

Uh oh!

PiDelport commented Mar 28, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fix: store_path should create missing directories, like load_path #60

fix: store_path should create missing directories, like load_path #60

Uh oh!

Conversation

PiDelport commented Mar 3, 2022

Uh oh!

matthiasbeyer left a comment

Choose a reason for hiding this comment

Uh oh!

matthiasbeyer Mar 3, 2022

Choose a reason for hiding this comment

Uh oh!

PiDelport Mar 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PiDelport commented Mar 28, 2022

Uh oh!

Dylan-DPC commented Mar 28, 2022

Uh oh!

PiDelport commented Mar 28, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fix: `store_path` should create missing directories, like `load_path` #60

fix: `store_path` should create missing directories, like `load_path` #60

PiDelport Mar 20, 2022 •

edited

Loading