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

fix/feat: add cfg module to get or create the .wash directory #216

Merged
merged 4 commits into from
Dec 13, 2021

Conversation

ghost
Copy link

@ghost ghost commented Dec 12, 2021

Fixes #215.

Currently if the .wash folder does not already exist, the ctx commands fail. This PR adds a cfg module along with a cfg_dir function that gets the .wash directory, and creates it if it does not exist.

This will need to be added to all of the functions that use the .wash directory. That looks like keys.rs and a few other ones in ctx. I'll implement these after getting a review on this design and any feedback you guys have.

Signed-off-by: Matt Wilkinson <mattwilki17@gmail.com>
@ghost ghost changed the title add cfg module to get/create the .wash directory fix/feat: add cfg module to get/create the .wash directory Dec 12, 2021
@ghost ghost changed the title fix/feat: add cfg module to get/create the .wash directory fix/feat: add cfg module to get or create the .wash directory Dec 12, 2021
src/ctx/mod.rs Show resolved Hide resolved
@ghost
Copy link
Author

ghost commented Dec 13, 2021

Follow up question @brooksmtownsend - Why do we have separate environment variables for each part of the config directory? Like WASH_CONTEXTS etc. Could we just have a WASH_CONFIG variable so they can customize where the config is stored, but not the layout inside the config folder?

My thinking is that if we then add more configuration options, users would have to maintain tons of environment variables if they wanted to customize where the config is stored, vs just setting a single one and calling it a day.

Matt Wilkinson added 3 commits December 13, 2021 13:04
Signed-off-by: Matt Wilkinson <mattwilki17@gmail.com>
Signed-off-by: Matt Wilkinson <mattwilki17@gmail.com>
Signed-off-by: Matt Wilkinson <mattwilki17@gmail.com>
@brooksmtownsend
Copy link
Member

Why do we have separate environment variables for each part of the config directory? Like WASH_CONTEXTS etc. Could we just have a WASH_CONFIG variable so they can customize where the config is stored, but not the layout inside the config folder?

The main reason here was that someone could feasibly set WASH_KEYS=/tmp/mykeys where they mounted a folder containing keys, and it wouldn't have to be called /path/to/folder/keys.

I think there's three possible scenarios for someone configuring wash

  1. User is fine with just using $HOME/.wash, no configuration needed (most common)
  2. User wants to use a custom directory for wash things, e.g. $HOME/github/wash_cfg/, in which case a WASH_CONFIG or WASH_CFG_DIR environment variable is best
  3. User wants to individually use a custom directory, e.g. a filesystem mounted folder, for keys or contexts like /home/washctxs

We currently support all three options with WASH_KEYS and WASH_CONTEXTS, and if we used a WASH_CFG_DIR variable we couldn't support option 3. For now I lean towards keeping _KEYS and _CONTEXTS even though it's a little more verbose just because it's more configurable, but I'm certainly be open to more discussion

@ghost
Copy link
Author

ghost commented Dec 13, 2021

We currently support all three options with WASH_KEYS and WASH_CONTEXTS, and if we used a WASH_CFG_DIR variable we couldn't support option 3. For now I lean towards keeping _KEYS and _CONTEXTS even though it's a little more verbose just because it's more configurable, but I'm certainly be open to more discussion

Ok that makes sense! Perhaps could revisit this if it becomes an issue later.

@ghost
Copy link
Author

ghost commented Dec 13, 2021

I've removed the home_dir function and made all references to .wash use the cfg_dir one. This PR should be ready now @brooksmtownsend

Copy link
Member

@brooksmtownsend brooksmtownsend left a comment

Choose a reason for hiding this comment

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

LGTM, thank you again @mattwilkinsonn !

@brooksmtownsend brooksmtownsend merged commit 7d0e031 into wasmCloud:main Dec 13, 2021
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.

[BUG] .wash folder not created, context commands error out
1 participant