Merge ~ec0/juju-lint:fix-snap-and-better-errors into juju-lint:master

Proposed by James Hebden
Status: Merged
Approved by: James Troup
Approved revision: 5ae370127a4908793759c30456811af625a2a85c
Merged at revision: 0a72707edef77d9e82b20fb36199a7baf43261c1
Proposed branch: ~ec0/juju-lint:fix-snap-and-better-errors
Merge into: juju-lint:master
Diff against target: 312 lines (+78/-64)
6 files modified
jujulint/cli.py (+21/-4)
jujulint/config.py (+10/-10)
jujulint/config_default.yaml (+18/-24)
jujulint/lint.py (+26/-21)
setup.py (+2/-5)
snap/snapcraft.yaml (+1/-0)
Reviewer Review Type Date Requested Status
James Troup Pending
Review via email: mp+384624@code.launchpad.net

Commit message

Fix the snap, as it was not including the actual juju-lint library.
Also add more and better error handling around running juju-lint without a configuration file.
Support both absolute and config-relative paths for lint rules.

To post a comment you must log in.
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

Change successfully merged at revision 0a72707edef77d9e82b20fb36199a7baf43261c1

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/jujulint/cli.py b/jujulint/cli.py
2index a75a7c3..68e7bb2 100755
3--- a/jujulint/cli.py
4+++ b/jujulint/cli.py
5@@ -21,8 +21,10 @@ from jujulint.config import Config
6 from jujulint.lint import Linter
7 from jujulint.logging import Logger
8 from jujulint.openstack import OpenStack
9+import os.path
10 import pkg_resources
11 import yaml
12+import sys
13
14
15 class Cli:
16@@ -35,9 +37,16 @@ class Cli:
17 self.config = Config()
18 self.logger = Logger(self.config["logging"]["loglevel"].get())
19 self.version = pkg_resources.require("jujulint")[0].version
20- self.lint_rules = "{}/{}".format(
21- self.config.config_dir(), self.config["rules"]["file"].get()
22- )
23+ rules_file = self.config["rules"]["file"].get()
24+ # handle absolute path provided
25+ if os.path.isfile(rules_file):
26+ self.lint_rules = rules_file
27+ elif os.path.isfile("{}/{}".format(self.config.config_dir(), rules_file)):
28+ # default to relative path
29+ self.lint_rules = "{}/{}".format(self.config.config_dir(), rules_file)
30+ else:
31+ self.logger.error("Cloud not locate rules file {}".format(rules_file))
32+ sys.exit(1)
33
34 def startup_message(self):
35 """Print startup message to log."""
36@@ -45,14 +54,20 @@ class Cli:
37 (
38 "juju-lint version {} starting...\n"
39 "\t* Config directory: {}\n"
40+ "\t* Rules file: {}\n"
41 "\t* Log level: {}\n"
42 ).format(
43 self.version,
44 self.config.config_dir(),
45+ self.lint_rules,
46 self.config["logging"]["loglevel"].get(),
47 )
48 )
49
50+ def usage(self):
51+ """Print program usage."""
52+ self.config.parser.print_help()
53+
54 def audit_file(self, filename, cloud_type=None):
55 """Directly audit a YAML file."""
56 self.logger.debug("Starting audit of file {}".format(filename))
57@@ -132,5 +147,7 @@ def main():
58 cli.audit_file(manual_file, cloud_type=manual_type)
59 else:
60 cli.audit_file(manual_file)
61- else:
62+ elif "clouds" in cli.config:
63 cli.audit_all()
64+ else:
65+ cli.usage()
66diff --git a/jujulint/config.py b/jujulint/config.py
67index b4d1e1c..fb5497a 100644
68--- a/jujulint/config.py
69+++ b/jujulint/config.py
70@@ -29,8 +29,8 @@ class Config(Configuration):
71 """Wrap the initialisation of confuse's Configuration object providing defaults for our application."""
72 super().__init__("juju-lint", __name__)
73
74- parser = ArgumentParser(description="Sanity check one or more Juju models")
75- parser.add_argument(
76+ self.parser = ArgumentParser(description="Sanity check one or more Juju models")
77+ self.parser.add_argument(
78 "-l",
79 "--log-level",
80 type=str,
81@@ -39,7 +39,7 @@ class Config(Configuration):
82 help="The default log level, valid options are info, warn, error or debug",
83 dest="logging.loglevel",
84 )
85- parser.add_argument(
86+ self.parser.add_argument(
87 "-d",
88 "--output-dir",
89 type=str,
90@@ -48,7 +48,7 @@ class Config(Configuration):
91 help="The folder to use when saving gathered cloud data and lint reports.",
92 dest="output.folder",
93 )
94- parser.add_argument(
95+ self.parser.add_argument(
96 "--dump-state",
97 type=str,
98 help=(
99@@ -57,14 +57,14 @@ class Config(Configuration):
100 ),
101 dest="output.dump",
102 )
103- parser.add_argument(
104+ self.parser.add_argument(
105 "-c",
106 "--config",
107 default="lint-rules.yaml",
108 help="File to read lint rules from. Defaults to `lint-rules.yaml`",
109 dest="rules.file",
110 )
111- parser.add_argument(
112+ self.parser.add_argument(
113 "manual-file",
114 metavar="manual-file",
115 nargs='?',
116@@ -75,7 +75,7 @@ class Config(Configuration):
117 "Setting this disables collection of data from remote or local clouds configured via config.yaml."
118 ),
119 )
120- parser.add_argument(
121+ self.parser.add_argument(
122 "-t",
123 "--cloud-type",
124 help=(
125@@ -83,13 +83,13 @@ class Config(Configuration):
126 ),
127 dest="manual-type",
128 )
129- parser.add_argument(
130+ self.parser.add_argument(
131 "-o",
132 "--override-subordinate",
133 dest="override.subordinate",
134 help="override lint-rules.yaml, e.g. -o canonical-livepatch:all",
135 )
136- parser.add_argument(
137+ self.parser.add_argument(
138 "--logfile",
139 "-L",
140 default=None,
141@@ -97,5 +97,5 @@ class Config(Configuration):
142 dest="logging.file",
143 )
144
145- args = parser.parse_args()
146+ args = self.parser.parse_args()
147 self.set_args(args, dots=True)
148diff --git a/jujulint/config_default.yaml b/jujulint/config_default.yaml
149index 19cbe91..662df79 100644
150--- a/jujulint/config_default.yaml
151+++ b/jujulint/config_default.yaml
152@@ -1,30 +1,24 @@
153 ---
154-clouds:
155- # an example local Juju-deployed OpenStack
156- cloud1:
157- type: openstack
158- access: local
159+# clouds:
160+# an example local Juju-deployed OpenStack
161+# cloud1:
162+# type: openstack
163+# access: local
164
165- # an example remote Juju-deployed OpenStack
166- cloud2:
167- type: openstack
168- access: ssh
169- host: 'ubuntu@openstack.example.fake'
170+# an example remote Juju-deployed OpenStack
171+# cloud2:
172+# type: openstack
173+# access: ssh
174+# host: 'ubuntu@openstack.example.fake'
175
176- # an exported bundle
177- yamlfile:
178- type: openstack
179- access: dump
180- file: 'export.yaml'
181-
182- # an example remote Juju-deployed OpenStack where
183- # Juju controllers are registered under user 'juju-user
184- # in this example cloud, so we sudo to that user
185- cloud3:
186- type: openstack
187- access: ssh
188- host: 'ubuntu@openstack2.example.fake'
189- sudo: 'juju-user'
190+# an example remote Juju-deployed OpenStack where
191+# Juju controllers are registered under user 'juju-user
192+# in this example cloud, so we sudo to that user
193+# cloud3:
194+# type: openstack
195+# access: ssh
196+# host: 'ubuntu@openstack2.example.fake'
197+# sudo: 'juju-user'
198
199 logging:
200 level: INFO
201diff --git a/jujulint/lint.py b/jujulint/lint.py
202index 92e56d6..a132541 100755
203--- a/jujulint/lint.py
204+++ b/jujulint/lint.py
205@@ -19,6 +19,7 @@
206 # along with this program. If not, see <http://www.gnu.org/licenses/>.
207 """Lint operations and rule processing engine."""
208 import collections
209+import os.path
210 import pprint
211 import re
212
213@@ -84,30 +85,34 @@ class Linter:
214
215 def read_rules(self):
216 """Read and parse rules from YAML, optionally processing provided overrides."""
217- with open(self.filename, "r") as yaml_file:
218- self.lint_rules = yaml.safe_load(yaml_file)
219- if self.overrides:
220- for override in self.overrides.split("#"):
221- (name, where) = override.split(":")
222- self.logger.info(
223- "[{}] [{}/{}] Overriding {} with {}".format(
224- self.cloud_name,
225- self.controller_name,
226- self.model_name,
227- name,
228- where,
229+ if os.path.isfile(self.filename):
230+ with open(self.filename, "r") as yaml_file:
231+ self.lint_rules = yaml.safe_load(yaml_file)
232+ if self.overrides:
233+ for override in self.overrides.split("#"):
234+ (name, where) = override.split(":")
235+ self.logger.info(
236+ "[{}] [{}/{}] Overriding {} with {}".format(
237+ self.cloud_name,
238+ self.controller_name,
239+ self.model_name,
240+ name,
241+ where,
242+ )
243 )
244+ self.lint_rules["subordinates"][name] = dict(where=where)
245+ self.lint_rules["known charms"] = flatten_list(self.lint_rules["known charms"])
246+ self.logger.debug(
247+ "[{}] [{}/{}] Lint Rules: {}".format(
248+ self.cloud_name,
249+ self.controller_name,
250+ self.model_name,
251+ pprint.pformat(self.lint_rules),
252 )
253- self.lint_rules["subordinates"][name] = dict(where=where)
254- self.lint_rules["known charms"] = flatten_list(self.lint_rules["known charms"])
255- self.logger.debug(
256- "[{}] [{}/{}] Lint Rules: {}".format(
257- self.cloud_name,
258- self.controller_name,
259- self.model_name,
260- pprint.pformat(self.lint_rules),
261 )
262- )
263+ return True
264+ self.logger.error("Rules file {} does not exist.".format(self.filename))
265+ return False
266
267 def process_subordinates(self, app_d, app_name):
268 """Iterate over subordinates and run subordinate checks."""
269diff --git a/setup.py b/setup.py
270index 4190e7a..b6b3dc9 100644
271--- a/setup.py
272+++ b/setup.py
273@@ -17,7 +17,6 @@
274 # along with this program. If not, see <http://www.gnu.org/licenses/>.
275 """Setuptools packaging metadata for juju-lint."""
276
277-import re
278 import setuptools
279 import warnings
280
281@@ -28,9 +27,7 @@ with open("README.md", "r") as fh:
282
283 setuptools.setup(
284 name="jujulint",
285- use_scm_version={
286- "local_scheme": "node-and-date",
287- },
288+ use_scm_version={"local_scheme": "node-and-date"},
289 author="Canonical",
290 author_email="juju@lists.ubuntu.com",
291 description="Linter for Juju models to compare deployments with configurable policy",
292@@ -45,7 +42,7 @@ setuptools.setup(
293 "Intended Audience :: System Administrators",
294 ],
295 python_requires=">=3.4",
296- py_modules=["jujulint"],
297+ packages=["jujulint"],
298 entry_points={"console_scripts": ["juju-lint=jujulint.cli:main"]},
299 setup_requires=["setuptools_scm"],
300 )
301diff --git a/snap/snapcraft.yaml b/snap/snapcraft.yaml
302index b8bd6da..9ec50ef 100644
303--- a/snap/snapcraft.yaml
304+++ b/snap/snapcraft.yaml
305@@ -22,6 +22,7 @@ parts:
306 requirements:
307 - requirements.txt
308 source: .
309+ source-type: git
310 override-build: |
311 snapcraftctl build
312 echo "Version: $(python3 setup.py --version)"

Subscribers

People subscribed via source and target branches