Add config file reading/writing by tsroten · Pull Request #39 · dbcli/cli_helpers

tsroten · 2018-02-13T03:46:05Z

Description

Adds config file reading. Client applications do not need to worry about where the config files are stored (unless they want to). Client applications can trust that the config files will be read safely and validated for the correct types/options. Booleans are booleans, integers are integers, etc.

A few notes:

This uses the XDG spec for macOS and Linux. For Windows, it will respect the typically-recommended Windows locations for config files.
This is easily extensible by client applications to add additional directories, customize the system directories, or change the user configuration file location.
This supports config file validation using Configobj's validation library. It also converts values to the correct type (e.g. boolean, integer, etc.)

Current documentation for this: http://cli-helpers.readthedocs.io/en/feature-config/api.html#module-cli_helpers.config

NOTE: more documentation is being worked on for this.

The Simplest Use Case

Here is the simplest use case, with no default config and no custom file locations:

from cli_helpers.config import Config

config = Config('myapp', 'My Organization', 'config')
config.read()

A Pretty Simple Use Case

Default config file:

[main]
# Default log level. Possible values: "CRITICAL", "ERROR", "WARNING", "INFO"
# and "DEBUG". "NONE" disables logging.
log_level = option('CRITICAL', 'ERROR', 'WARNING', 'INFO', 'DEBUG', 'NONE', default='INFO')

Client code:

import os

from cli_helpers.config import Config

PACKAGE_ROOT = os.path.dirname(__file__)

config = Config('mycli', 'dbcli', 'config', default=os.path.join(PACKAGE_ROOT, self.filename)
                validate=True)
config.read()

print(config['main']['log_level'])

In that case, the log level is guaranteed to be a valid option and it defaults to 'INFO'.

Default Config Values

Default values can be specified as a dict or as a path to a config file.

config = Config('myapp', 'My Organization', 'config', default={'main': {'debug': False}})

config = Config('myapp', 'My Organization', 'config', default='/path/to/default/file')

Checklist

I've added this contribution to the CHANGELOG.
I've added my name to the AUTHORS file (or it's already there).

…nfig

…nfig * 'master' of github.com:dbcli/cli_helpers: Pass CODECOV environment variable to tox. Do not trigger extra builds with codecov. Use CODECOV environment variable instead of TRAVIS. Fix markup format test. Fix kwargs in tabulate_adapter. Do coverage xml before uploading to codecov. Pass coverage file name to codecov. Coverage for tox tests Add coverage file to gitignore. Add -c argument to tox tests. Increase test coverage. Remove try/except that should never be hit. Test format_output() wrapper. Add coverage to changelog. Do not combine coverage. Add coverage badge. Do not count packages in coverage. Do not use parallel coverage. Fix codecov. Add codecov.

…nfig * 'master' of github.com:dbcli/cli_helpers: Test custom token names. Clarify skipif reason. Add test that tabulate handles ANSI escapes correctly. Remove missingval. Use override_missing_value for tabulate. Fix tox tests for noextras. Add styled output to changelog. Add style_output preprocessor. Add extra_requires for styles.

…nfig * 'master' of github.com:dbcli/cli_helpers: Update appveyor badge/link to be org project. Add appveyor to changelog. Add appveyor badge to readme. Upload appveyor coverage to codecov. Set PATH so it's easier to run things. Invoke pytest as a module. Use pytest, not py.test. Do not lint for windows tests. Add appveyor to check-manifest ignore. Add appveyor file.

…nfig * 'master' of github.com:dbcli/cli_helpers: (25 commits) Bump version for v0.2.0 release. Remove import decimal. Align floats as well. Test non-decimals in align_decimals function. Don't include number formatting by default. Reverse align_decimal changes. Add tests for is_number. Make align_decimals operate on strings. Use format_numbers for terminaltables and vertical table. Add format_numbers to changelog. Add format_numbers docstring. Use integer instead of decimal so it's not confusing. Remove unneeded "d". Add test for single format string. Add test for _get_type. Test full return values. Test that formatting doesn't happen without format strings/types. Remove u string prefix. Add docstrings. Move float and int types to compat.py. ...

This reverts commit 8f1b437.

…nfig * 'master' of github.com:dbcli/cli_helpers: Add unicode fix to change log. Test non-ascii on style output. Fix stringio and csv unicode problems. Add export to path variables. Set -ex together. Add mac tests to changelog. Remove extra install virtualenv. activate virtualenv before tox. Use virtualenv. Ignore .travis in manifest check. Add osx tests. Don't use venv. Use travis install script. Add install script for travis to use.

…nfig * 'master' of github.com:dbcli/cli_helpers: (21 commits) Bump version to 0.2.3. Do not add extra adapter in test. Pass column_types on unmodified. Unpack format_numbers result before testing assertion. fixup! fixes to accept iterator fixup! fixes to accept iterator fixup! fixes to accept iterator fixup! fixes to accept iterator fixup! fixes to accept iterator fixup! fixes to accept iterator fixup! fixes to accept iterator Add unicode error Python 2 fix to changelog. Add non-ascii text to newline test. Use compat module for StringIO. Add styling test with newlines in it. Bump version to v0.2.2. Add uneven row fix to changelog. Use itertools to get column types. fixup! fixes to accept iterator fixes to accept iterator ...

…nfig * 'master' of github.com:dbcli/cli_helpers: Do not test travis macos py27. Copy unit test from pgcli Releasing version v1.0.1. Update changelog for v1.0.1 release. Output all unicode for terminaltables, add unit test Release version 1.0.0 Bump changelog version to v1.0.0 Remove tasks entry points. fixup! Drop Python 3.3. Use backports.csv only for py2 Drop Python 3.3. Update changelog for tabulate. Remove vendored tabulate module. Use tabulate from PyPI. Output as generator

meeuw · 2018-02-15T20:20:08Z

Hi Thomas, it looks all good, I'd like to know why you've chosen to split the code the way you did.
For example; would it be a good idea to merge:

Config.read_default_config / Config.read
Config._set_default / Config.__init__
Config._raise_validation_errors / Config.read_default_config
Config.all_config_files / Config.system_config_files / Config.additional_files
Config.read / Config.all_config_files
Config.read / Config.read_config_files

This would certainly improve readability, but if you already know all of these functions will directly used from mycli/pgcli/mssql-cli we should keep them (of course) seperate.

tsroten · 2018-02-17T16:29:47Z

@meeuw Thanks for the helpful feedback! I appreciate the good questions.

I personally prefer a maximum of two-to-three levels deep of conditional/loop indentation in functions, with large blocks of code within loops being split into functions, which is why some of that is broken out the way it is. I feel like it's easier to read indented code blocks when they are put in named functions.

I think some of the ones you mentioned make sense to merge. I'll try to rework these a bit so it's not as spread out:

_set_default merged into __init__
_raise_validation_errors into read_default_config

The other short file-based ones (all_config_files, system_config_files, user_config_file, and additional_files) make it very easy for client applications to override one or two pieces of the default configuration setup without affecting the rest. That might be my day-to-day Ruby programming shining through a bit.

Ultimately, my goal in this API was two-fold:

Make the default interface extremely easy-to-use (create a config object, then call read()).
Don't assume all client applications handle configuration the same way, so make it easy for them to customize various parts of the configuration process.

So, for number 1, we end up with one client-facing method that is the primary way to load the configuration -- read().

For number 2, I tried to make it so client applications wouldn't have to replicate a whole lot of the logic if they want to override different portions of the config reading/writing:

Which system config files to read
Which user config file to read
Which additional files to read (also controllable via the constructor)
The reading of the files themselves
The reading of the default config file (which might need different logic than the user-facing config files)
The writing of the default config file
The writing of the current, in-memory config to a file

jayvdb · 2018-04-07T02:03:59Z

Why not use https://github.com/ActiveState/appdirs to obtain the relevant directory paths when that is desired by the caller?

As Windows has so many changes to their dirs, you are begging for support problems by reimplementing this.

If appdirs doesnt provide the information in a usable way, maybe we can enhance appdirs so it does work for the needs of dbcli ...?

amjith · 2018-04-07T03:08:39Z

@jayvdb I believe the original reason was, we wanted a standard place to put our config files so we can tell users to go to a specific location on their computer and edit the config file.

appdirs will give you the appropriate directory for the current operating system which is different for Windows 7, 8, 10 etc. Which makes our life harder when we want them to enable/disable a feature via the config file.

jayvdb · 2018-04-07T03:25:19Z

Ya, I guessed that, but you can use appdirs to give you the locations (any of them), and you choose which one you want to use.

It leads to better behaviour on flavours of Unix which you dont want to even know about, but their users are rather finicky about how apps work ;-)

amjith · 2018-04-07T03:36:20Z

The same reason applies to the Unix folks as well. We want our config files in a specific place so we can easily guide people there.

We try to respect the standards wherever possible, for example, the config file used to go in the home folder ~/.pgclirc. We changed that to ~/.config/pgcli/config when we realized that it is disrespectful to litter the user's home folder.

But we're trying to balance correctness and convenience. This is the best compromise we came up with, if you have examples of this being insufficient, I'm happy to consider it.

tsroten · 2018-04-08T12:25:07Z

appdirs is really pretty handy. @amjith is definitely right about balancing correctness and convenience.

Another specific use case is for macOS. In my experience, CLI apps users on Mac expect the config to behave like Linux (i.e. XDG). But, appdirs doesn't support XDG-based directories on macOS.

…nfig * 'master' of github.com:dbcli/cli_helpers: Releasing version 1.0.2. Move strip_ansi from tests.utils to cli_helpers.utils Fix pep8 issue in build. use safe float (2578310786669609 / pow(2, 50) == 2.29)

codecov-io · 2018-05-27T20:34:13Z

Codecov Report

Merging #39 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff           @@
##           master    #39    +/-   ##
======================================
  Coverage     100%   100%            
======================================
  Files          10     11     +1     
  Lines         280    395   +115     
======================================
+ Hits          280    395   +115

Impacted Files	Coverage Δ
cli_helpers/compat.py	`100% <100%> (ø)`	⬆️
cli_helpers/config.py	`100% <100%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b63f54b...8a8fda9. Read the comment docs.

tsroten · 2018-05-27T20:38:19Z

@meeuw Okay, I've combined some of the functions together to make the logic more readable. Thanks!

I've kept some of them separate because I do think it will be helpful as a general-use API so that people can override the various behaviors as they need to.

amjith · 2018-10-09T03:52:22Z

Can we merge this PR and release a new version?

tsroten and others added 30 commits April 21, 2017 22:14

Add configobj dependency.

241a3a0

Merge branch 'master' of github.com:dbcli/cli_helpers into feature/co…

8113d0e

…nfig

Add windows constant and UserDict to compat module.

5daa0ba

Add config directory functions.

4711a03

Merge branch 'master' of github.com:dbcli/cli_helpers into feature/co…

eb37880

…nfig

Initial config reading commit.

b0606f3

Move test utils to separate module.

df8c5c9

Add mock for Python 2.

e1f413c

Fix Python 2 tests.

c20e5be

Remove unused fake file.

2576327

Remove unused module.

9ec7fd5

Add MAC to compat module.

1bd0c98

Add test for overwriting user config file.

9cd0cc3

Test user config filename.

1167984

Add second section to test config.

0f35ae8

Test reading invalid config file.

1d58a4b

Allow for inheriting from Config class.

4ab5089

Add Config.write() method.

2a7a9b3

Use temp dir for test file.

b83aec8

Don't read config file automatically.

1e3b332

Add tests/config_data to manifest file.

bf9215b

Refactor raising validation errors.

0dab0e4

Shorten the write function.

03433f8

Shorten write method even more.

1be63b4

Don't call expanduser unnecessarily.

fb88517

Break out setting default.

5236c11

Add macOS to travis.

8f1b437

tsroten and others added 12 commits June 23, 2017 16:17

Revert "Add macOS to travis."

dab3841

This reverts commit 8f1b437.

Add more docstrings.

449d07f

Initial doc work for config.

4c846dc

Fix config test.

61f56d0

correct docstring.

401db81

Update changelog.

e39e685

Update readme.

192e991

Fix autopep8 bug

fbd035c

tsroten mentioned this pull request Feb 13, 2018

Config file revamp dbcli/mycli#581

Closed

9 tasks

Merge branch 'master' of github.com:dbcli/cli_helpers into feature/co…

20c637d

…nfig * 'master' of github.com:dbcli/cli_helpers: Releasing version 1.0.2. Move strip_ansi from tests.utils to cli_helpers.utils Fix pep8 issue in build. use safe float (2578310786669609 / pow(2, 50) == 2.29)

Combine some of the code into the same methods.

8a8fda9

tsroten merged commit 90276cd into master Oct 18, 2018

tsroten deleted the feature/config branch October 18, 2018 11:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add config file reading/writing#39

Add config file reading/writing#39
tsroten merged 44 commits intomasterfrom
feature/config

tsroten commented Feb 13, 2018 •

edited

Loading

Uh oh!

meeuw commented Feb 15, 2018 •

edited

Loading

Uh oh!

tsroten commented Feb 17, 2018

Uh oh!

jayvdb commented Apr 7, 2018

Uh oh!

amjith commented Apr 7, 2018

Uh oh!

jayvdb commented Apr 7, 2018

Uh oh!

amjith commented Apr 7, 2018

Uh oh!

tsroten commented Apr 8, 2018

Uh oh!

codecov-io commented May 27, 2018 •

edited

Loading

Uh oh!

tsroten commented May 27, 2018

Uh oh!

amjith commented Oct 9, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

tsroten commented Feb 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

The Simplest Use Case

A Pretty Simple Use Case

Default Config Values

Checklist

Uh oh!

meeuw commented Feb 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tsroten commented Feb 17, 2018

Uh oh!

jayvdb commented Apr 7, 2018

Uh oh!

amjith commented Apr 7, 2018

Uh oh!

jayvdb commented Apr 7, 2018

Uh oh!

amjith commented Apr 7, 2018

Uh oh!

tsroten commented Apr 8, 2018

Uh oh!

codecov-io commented May 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

tsroten commented May 27, 2018

Uh oh!

amjith commented Oct 9, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

tsroten commented Feb 13, 2018 •

edited

Loading

meeuw commented Feb 15, 2018 •

edited

Loading

codecov-io commented May 27, 2018 •

edited

Loading