Comments (5)
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.
Related Issues (20)
- Best practice to read configuration values from whithin commands? HOT 1
- What is client_factory?
- util.todict process DefaultInt to a dictionary instead of a int HOT 1
- CLICommandsLoader.create_command overrides the argument "arguments_loader" HOT 3
- Release candidate tags are sorted as higher than actual releases (0.8.0rc2 >> 0.8.0) HOT 7
- TSV output doesn't include headers
- Misleading error message for invalid choice arguments
- Knack error when I run azdev linter
- How to disable default Global Arguments (especially -o)?
- When json value has null, knack in Azure CLI changes column order unintentionally. HOT 1
- Colorama is not being installed in Windows even though it is required
- I think Knack is pulling a version of PyYAML that has a known vulnerability. HOT 10
- Config file value ignored by command validation code HOT 1
- This repo is missing important files
- Set column width for tabular output format HOT 1
- `extract_full_summary_from_signature` uses undocumented `Match.regs`
- Inefficient regex in `extract_full_summary_from_signature`
- knack/knack /prompting.py _prompt_bool never taking default action
- Broken `examples/test_exapp`: TypeError: ScenarioTest.__init__() missing 1 required positional argument: 'method_name' HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from knack.