Skip to content

Commit

Permalink
Combine bug fix, improve exception & file handling
Browse files Browse the repository at this point in the history
- combine: Force sort order for combine source files (don't rely on directory
  sort order from a os.walk() command, which is OS or filesytem dependent)
  This issue abruptly showed up via TravisCI (on a minor doc change commit).
  Travis runs on Ubuntu (ext4).  Not an issue on files systems that internally
  sort read_dir() output, however this should NOT be relied upon.  Adding an
  explicit sort is the right thing to do for all OSes.
- combine: Add a proper try/finally block around the conf merge handler.
  Previously still seeing an "unclosed file"  resource warning periodically
  during unit tests.
- Added missing exit code checks to one of the 'combine' unit tests failing for
  Travis.  (Unable to reproduce locally; on a Mac)
- combine:  Use the word 'directory' vs 'folder' uniformly in user output.
- Bump pre-commit-hooks to latest 1.x release (skipping 2.0 for now)
- Add issue templates for GitHub (first pass)
  • Loading branch information
lowell80 committed Nov 1, 2018
1 parent fbd8628 commit 5052d01
Show file tree
Hide file tree
Showing 5 changed files with 100 additions and 16 deletions.
49 changes: 49 additions & 0 deletions .github/ISSUE_TEMPLATE/bug.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
---
name: Bug report
about: Report an issue or problem

---

# The problem

Briefly describe the issue you are experiencing. Tell us what you were trying to do and what happened instead. If you have a question, discard this template and please use the "question" label.

## Environment

* Ksconf version: (Grab the first 2 text lines of output after running `ksconf --version`)
* OS & version used:
* Python version:
* Installed via: (pip, git, standalone zip, Splunk's embedded python)

## Details

Describe the problem you have been experiencing in more detail. Please include the command line arguments and output text. Stacktraces are especially useful. Any steps you've take to troubleshoot or hints about what's wrong in the code can be very informative.


Wrap CLI output in triple backticks, like so:

```
$ ksconf --version
_ ___
| | / __)
| | _ ___ ____ ___ ____ _| |__
| |_/ )/___)/ ___) _ \| _ (_ __)
| _ (|___ ( (__| |_| | | | || |
|_| \_|___/ \____)___/|_| |_||_|
ksconf 0.5.1.dev0+dirty (Build None)
Git SHA1 17f6d94b committed on 2018-06-28
Written by Lowell Alleman <lowell@kintyre.co>.
Copyright (c) 2018 Kintyre Solutions, Inc.
Licensed under Apache Public License v2
```


## Steps To Reproduce Issue [ Good To Have ]

Please remember that sample configs often make problems easier to reproduce making it faster to fix the bug.

1. Step 1
2. Step 2
3. Step 3

25 changes: 25 additions & 0 deletions .github/ISSUE_TEMPLATE/feature-request.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
---
name: Feature request
about: Suggest features or changes to ksconf

---

## Is your feature request related to a problem? Please describe.

A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]

## Describe the solution you'd like

A clear and concise description of the analytics or functionality you'd like to see in in ksconf.

## Describe alternatives you've considered

A clear and concise description of any alternative solutions or features you've considered. Is there another way to solve or detect the same problem?

## How serious are you about this change? [ For not-trivial changes ]

Is this a nice-to-have or showstopper for your use case? Are you will and able to contibute code, time, or resources to make this change happen?

## Additional information

Add any other context, screenshots, diagrams, mock-ups, and so on should be placed here.
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
# See http://pre-commit.com/hooks.html for more hooks
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
sha: v1.3.0
sha: v1.4.0
hooks:
- id: trailing-whitespace
exclude: \.md$
Expand Down
38 changes: 23 additions & 15 deletions ksconf/commands/combine.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,10 +161,10 @@ def run(self, args):
self.stderr.write("Must provide the '--target' directory.\n")
return EXIT_CODE_MISSING_ARG

self.stderr.write("Combining conf files into {}\n".format(args.target))
self.stderr.write("Combining conf files into directory {}\n".format(args.target))
args.source = list(_expand_glob_list(args.source))
for src in args.source:
self.stderr.write("Reading conf files from {}\n".format(src))
self.stderr.write("Reading conf files from directory {}\n".format(src))

marker_file = os.path.join(args.target, CONTROLLED_DIR_MARKER)
if os.path.isdir(args.target):
Expand All @@ -174,9 +174,9 @@ def run(self, args):
return EXIT_CODE_COMBINE_MARKER_MISSING
elif args.dry_run:
self.stderr.write(
"Skipping creating destination folder {0} (dry-run)\n".format(args.target))
"Skipping creating destination directory {0} (dry-run)\n".format(args.target))
else:
self.stderr.write("Creating destination folder {0}\n".format(args.target))
self.stderr.write("Creating destination directory {0}\n".format(args.target))
os.mkdir(args.target)
open(marker_file, "w").write("This directory is managed by KSCONF. Don't touch\n")

Expand Down Expand Up @@ -204,6 +204,8 @@ def run(self, args):
target_extra_files.add(tgt_file)

for (dest_fn, src_files) in sorted(src_file_index.items()):
# Source file must be in sort order (10-x is lower prio and therefore replaced by 90-z)
src_files = sorted(src_files)
dest_path = os.path.join(args.target, dest_fn)

# Make missing destination folder, if missing
Expand Down Expand Up @@ -235,17 +237,23 @@ def run(self, args):
self.stderr.write(
"Copy <{0}> {1:50} from {2}\n".format(smart_rc, dest_path, src_file))
else:
# Handle merging conf files
dest = ConfFileProxy(os.path.join(args.target, dest_fn), "r+",
parse_profile=PARSECONF_MID)
srcs = [ConfFileProxy(sf, "r", parse_profile=PARSECONF_STRICT) for sf in src_files]
# self.stderr.write("Considering {0:50} CONF MERGE from source: {1!r}\n".format(dest_fn, src_files[0]))
smart_rc = merge_conf_files(dest, srcs, dry_run=args.dry_run,
banner_comment=args.banner)
if smart_rc != SMART_NOCHANGE:
self.stderr.write(
"Merge <{0}> {1:50} from {2!r}\n".format(smart_rc, dest_path,
src_files))
try:
# Handle merging conf files
dest = ConfFileProxy(os.path.join(args.target, dest_fn), "r+",
parse_profile=PARSECONF_MID)
srcs = [ConfFileProxy(sf, "r", parse_profile=PARSECONF_STRICT) for sf in src_files]
# self.stderr.write("Considering {0:50} CONF MERGE from source: {1!r}\n".format(dest_fn, src_files[0]))
smart_rc = merge_conf_files(dest, srcs, dry_run=args.dry_run,
banner_comment=args.banner)
if smart_rc != SMART_NOCHANGE:
self.stderr.write(
"Merge <{0}> {1:50} from {2!r}\n".format(smart_rc, dest_path,
src_files))
finally:
# Protect against any dangling open files: (ResourceWarning: unclosed file)
dest.close()
for src in srcs:
src.close()

if True and target_extra_files: # Todo: Allow for cleanup to be disabled via CLI
self.stderr.write("Cleaning up extra files not part of source tree(s): {0} files.\n".
Expand Down
2 changes: 2 additions & 0 deletions tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,7 @@ def test_combine_3dir(self):
default = twd.get_path("etc/apps/Splunk_TA_aws/default")
with ksconf_cli:
ko = ksconf_cli("combine", "--target", default, default + ".d/*")
self.assertEqual(ko.returncode, EXIT_CODE_SUCCESS)
cfg = parse_conf(twd.get_path("etc/apps/Splunk_TA_aws/default/props.conf"))
self.assertIn("aws:config", cfg)
self.assertEqual(cfg["aws:config"]["ANNOTATE_PUNCT"], "true")
Expand All @@ -297,6 +298,7 @@ def test_combine_3dir(self):
twd.write_file("etc/apps/Splunk_TA_aws/default/data/dead.conf", "# File to remove")
with ksconf_cli:
ko = ksconf_cli("combine", "--dry-run", "--target", default, default + ".d/*")
self.assertEqual(ko.returncode, EXIT_CODE_SUCCESS)
self.assertRegex(ko.stdout, r'[\r\n][-]\s*<view name="search"')
self.assertRegex(ko.stdout, r"[\r\n][+]TIME_FORMAT = [^\r\n]+%6N")
with ksconf_cli:
Expand Down

0 comments on commit 5052d01

Please sign in to comment.