Remove / move `ensure_dir` call since it might cause some issues and not consistent about knack HOT 5 OPEN

Removing the check altogether may make sense.

One consideration is ensure_dir(config_dir) may have been added in CLIConfig#init to help keep the check central vs. needing to have to check if the directory exists in multiple other places. For example, if someone is storing other files in the same config directory and wants to access those files directly and outside of the config methods provided by this library. Without the central ensure_dir(...) they'd have to handle this check themselves each time beforehand.

Removing that line could be a breaking change for people that depended on this library doing that check for them. Azure CLI does in-fact do its own check anyway though - https://github.com/Azure/azure-cli/blob/aee788b9c61b7982a5c8e7807d86d764c7f7a798/src/azure-cli-core/azure/cli/core/__init__.py#L78C31-L78C31.

Just something else to consider.

from knack.

Btw, I can try to contribute a fix back. Just curious about the bigger picture / context so that we don't break some upstream dependencies that already expect this behavior (why?).

from knack.

Disclaimer: It's been several years since I last worked on this project.

However, reading through your analysis, I'm not sure why in https://github.com/microsoft/knack/pull/91/files, it was added where it was. Yes it seems it'd be best to have the call to ensure_dir() in CLIConfig#init happen a few lines down after self.config_dir is defined since that's the directory that will be used.

from knack.

Yes it seems it'd be best to have the call to ensure_dir() in CLIConfig#init happen a few lines down after self.config_dir is defined since that's the directory that will be used.

Thanks @derekbekoe ! That's right it would be better (I think). But I don't quite understand the need for it anyways ... it would be still breaking some scenarios even a few lines below AFAI can tell.

from knack.

Thanks @derekbekoe for the research. Since it's a small change I would leave this decision to maintainers (I can do a PR if that makes sense).

Azure CLI does in-fact do its own check anyway though

I've seen that it is also using os.path.expanduser(os.path.join('~', '.{}'.format('cli'))) -like logic to determine the default path, but I didn't see that call. It means most likely that it'll keep breaking in the environments where home dir doesn't exist.

E.g. I've tried pip install in the Dcoker that has something like:

RUN  addgroup --system dvc && \
         adduser --system  --ingroup dvc dvc && \
USER dvc

And even pip install was breaking for me with a similar error. I didn't check the full stack trace though.

from knack.