Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Config File Does not Expand Environment Variables #3839

Closed
JoshMayberry opened this issue Sep 17, 2020 · 0 comments · Fixed by #4812
Closed

Config File Does not Expand Environment Variables #3839

JoshMayberry opened this issue Sep 17, 2020 · 0 comments · Fixed by #4812
Labels
Bug 🪲 Good first issue Friendly and approachable by new contributors Help wanted 🙏 Outside help would be appreciated, good for new contributors
Milestone
2.10.0

Comments

@JoshMayberry
Copy link

If a config file is provided that has an environment variable, such as "%AppData%", it fails.
This can be fixed by changing this line from config_file = os.path.expanduser(config_file) to config_file = os.path.expandvars(os.path.expanduser(config_file))
@PCManticore PCManticore added Bug 🪲 Good first issue Friendly and approachable by new contributors labels Dec 27, 2020
@Pierre-Sassoulas Pierre-Sassoulas added the Help wanted 🙏 Outside help would be appreciated, good for new contributors label Mar 2, 2021
PaaEl added a commit to PaaEl/pylint that referenced this issue Aug 7, 2021
@PaaEl
Update option_manager_mixin.py
617999a 
I  added @JoshMayberry 's solution for issue 3839.
pylint-dev#3839
@PaaEl PaaEl mentioned this issue Aug 7, 2021
Merged
@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.10.0 milestone Aug 7, 2021
Pierre-Sassoulas pushed a commit that referenced this issue Aug 11, 2021
@PaaEl
Update option_manager_mixin.py - issue 3839 (#4812)
d53a01f 
* Update option_manager_mixin.py

I  added user 'JoshMayberry' 's solution for issue 3839.
#3839

I  added a test to see if pylint handles environment variables in config file locations.
Added it to test_config because it also deals with config files.
To test I added an environment variable containing tmp_path and then unpacked it with os.path.expandvars.

Tmp_path gets converted to an environment variable  which is fed to the read_config_file function to be tested.
Read_config_file succesfully reads the environment variable, if the variable is changed, the function raises an OSError.
Pierre-Sassoulas added a commit that referenced this issue Nov 9, 2021
@Pierre-Sassoulas
Simplify 'test_can_read_toml_env_variable' to work like other tests
6c1d114 
See initial intent in #3839
This permit to not use the OptionManagerMixin directly, which is
problematic when it needs the function defined in Pylinter or
other classes.
@Pierre-Sassoulas Pierre-Sassoulas mentioned this issue Nov 9, 2021
Merged
Pierre-Sassoulas added a commit that referenced this issue Nov 9, 2021
@Pierre-Sassoulas
Simplify 'test_can_read_toml_env_variable' to work like other tests
749a7f3 
See initial intent in #3839
This permit to not use the OptionManagerMixin directly, which is
problematic when it needs the function defined in Pylinter or
other classes.
Pierre-Sassoulas added a commit that referenced this issue Nov 9, 2021
@Pierre-Sassoulas
Simplify 'test_can_read_toml_env_variable' to work like other tests
106ac13 
See initial intent in #3839
This permit to not use the OptionManagerMixin directly, which is
problematic when it needs the function defined in Pylinter or
other classes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug 🪲 Good first issue Friendly and approachable by new contributors Help wanted 🙏 Outside help would be appreciated, good for new contributors
Projects
None yet
Milestone
2.10.0
Development

Successfully merging a pull request may close this issue.

Expand environnement variable in option_manager_mixin PaaEl/pylint
3 participants
@PCManticore @Pierre-Sassoulas @JoshMayberry