Merge ~jfguedez/juju-lint:refactor-header-logging into juju-lint:master
- Git
- lp:~jfguedez/juju-lint
- refactor-header-logging
- Merge into master
Proposed by
Jose Guedez
Status: | Merged |
---|---|
Approved by: | James Troup |
Approved revision: | d219e5ebd839c1ff6b635a07b1b689a4cdbc378a |
Merged at revision: | bd261d5c562a1e3d32b68bd59b0b3739807ab693 |
Proposed branch: | ~jfguedez/juju-lint:refactor-header-logging |
Merge into: | juju-lint:master |
Diff against target: |
582 lines (+95/-247) 2 files modified
jujulint/lint.py (+91/-247) jujulint/logging.py (+4/-0) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Juju Lint maintainers | Pending | ||
Review via email: mp+408783@code.launchpad.net |
Commit message
Refactor logging to make code more readable
Description of the change
To post a comment you must log in.
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote : | # |
Revision history for this message
James Troup (elmo) wrote : | # |
Nice cleanup, thanks.
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote : | # |
Change successfully merged at revision bd261d5c562a1e3
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | diff --git a/jujulint/lint.py b/jujulint/lint.py | |||
2 | index 38dfc00..68fef78 100755 | |||
3 | --- a/jujulint/lint.py | |||
4 | +++ b/jujulint/lint.py | |||
5 | @@ -21,6 +21,7 @@ | |||
6 | 21 | import collections | 21 | import collections |
7 | 22 | from datetime import datetime, timezone | 22 | from datetime import datetime, timezone |
8 | 23 | import json | 23 | import json |
9 | 24 | import logging | ||
10 | 24 | import os.path | 25 | import os.path |
11 | 25 | import pprint | 26 | import pprint |
12 | 26 | import re | 27 | import re |
13 | @@ -126,27 +127,16 @@ class Linter: | |||
14 | 126 | if self.overrides: | 127 | if self.overrides: |
15 | 127 | for override in self.overrides.split("#"): | 128 | for override in self.overrides.split("#"): |
16 | 128 | (name, where) = override.split(":") | 129 | (name, where) = override.split(":") |
25 | 129 | self.logger.info( | 130 | self._log_with_header( |
26 | 130 | "[{}] [{}/{}] Overriding {} with {}".format( | 131 | "Overriding {} with {}".format(name, where), level=logging.INFO |
19 | 131 | self.cloud_name, | ||
20 | 132 | self.controller_name, | ||
21 | 133 | self.model_name, | ||
22 | 134 | name, | ||
23 | 135 | where, | ||
24 | 136 | ) | ||
27 | 137 | ) | 132 | ) |
28 | 138 | self.lint_rules["subordinates"][name] = dict(where=where) | 133 | self.lint_rules["subordinates"][name] = dict(where=where) |
29 | 139 | 134 | ||
30 | 140 | # Flatten all entries (to account for nesting due to YAML anchors (templating) | 135 | # Flatten all entries (to account for nesting due to YAML anchors (templating) |
31 | 141 | self.lint_rules = {k: flatten_list(v) for k, v in self.lint_rules.items()} | 136 | self.lint_rules = {k: flatten_list(v) for k, v in self.lint_rules.items()} |
32 | 142 | 137 | ||
40 | 143 | self.logger.debug( | 138 | self._log_with_header( |
41 | 144 | "[{}] [{}/{}] Lint Rules: {}".format( | 139 | "Lint Rules: {}".format(pprint.pformat(self.lint_rules)) |
35 | 145 | self.cloud_name, | ||
36 | 146 | self.controller_name, | ||
37 | 147 | self.model_name, | ||
38 | 148 | pprint.pformat(self.lint_rules), | ||
39 | 149 | ) | ||
42 | 150 | ) | 140 | ) |
43 | 151 | return True | 141 | return True |
44 | 152 | self.logger.error("Rules file {} does not exist.".format(self.filename)) | 142 | self.logger.error("Rules file {} does not exist.".format(self.filename)) |
45 | @@ -165,15 +155,7 @@ class Linter: | |||
46 | 165 | subordinates = [i.split("/")[0] for i in subordinates] | 155 | subordinates = [i.split("/")[0] for i in subordinates] |
47 | 166 | else: | 156 | else: |
48 | 167 | subordinates = [] | 157 | subordinates = [] |
58 | 168 | self.logger.debug( | 158 | self._log_with_header("{}: {}".format(unit, subordinates)) |
50 | 169 | "[{}] [{}/{}] {}: {}".format( | ||
51 | 170 | self.cloud_name, | ||
52 | 171 | self.controller_name, | ||
53 | 172 | self.model_name, | ||
54 | 173 | unit, | ||
55 | 174 | subordinates, | ||
56 | 175 | ) | ||
57 | 176 | ) | ||
59 | 177 | machine = app_d["units"][unit]["machine"] | 159 | machine = app_d["units"][unit]["machine"] |
60 | 178 | self.model.subs_on_machines.setdefault(machine, set()) | 160 | self.model.subs_on_machines.setdefault(machine, set()) |
61 | 179 | for sub in subordinates: | 161 | for sub in subordinates: |
62 | @@ -214,11 +196,8 @@ class Linter: | |||
63 | 214 | """Check if value is set per rule constraints.""" | 196 | """Check if value is set per rule constraints.""" |
64 | 215 | if rule in config: | 197 | if rule in config: |
65 | 216 | if check_value is True: | 198 | if check_value is True: |
71 | 217 | self.logger.debug( | 199 | self._log_with_header( |
72 | 218 | "[{}] [{}/{}] (PASS) Application {} correctly has config for '{}': {}.".format( | 200 | "(PASS) Application {} correctly has config for '{}': {}.".format( |
68 | 219 | self.cloud_name, | ||
69 | 220 | self.controller_name, | ||
70 | 221 | self.model_name, | ||
73 | 222 | name, | 201 | name, |
74 | 223 | rule, | 202 | rule, |
75 | 224 | config[rule], | 203 | config[rule], |
76 | @@ -241,11 +220,8 @@ class Linter: | |||
77 | 241 | ) | 220 | ) |
78 | 242 | return False | 221 | return False |
79 | 243 | elif check_value is False: | 222 | elif check_value is False: |
85 | 244 | self.logger.debug( | 223 | self._log_with_header( |
86 | 245 | "[{}] [{}/{}] (PASS) Application {} correctly had no config for '{}'.".format( | 224 | "(PASS) Application {} correctly had no config for '{}'.".format( |
82 | 246 | self.cloud_name, | ||
83 | 247 | self.controller_name, | ||
84 | 248 | self.model_name, | ||
87 | 249 | name, | 225 | name, |
88 | 250 | rule, | 226 | rule, |
89 | 251 | ) | 227 | ) |
90 | @@ -313,22 +289,16 @@ class Linter: | |||
91 | 313 | self, operator, app_name, check_value, config_key, app_config | 289 | self, operator, app_name, check_value, config_key, app_config |
92 | 314 | ): | 290 | ): |
93 | 315 | """Apply the provided config operator to the configuration.""" | 291 | """Apply the provided config operator to the configuration.""" |
94 | 316 | model_header = "[{}] [{}/{}]".format( | ||
95 | 317 | self.cloud_name, | ||
96 | 318 | self.controller_name, | ||
97 | 319 | self.model_name, | ||
98 | 320 | ) | ||
99 | 321 | |||
100 | 322 | # First check if the config key is present | 292 | # First check if the config key is present |
101 | 323 | if config_key not in app_config: | 293 | if config_key not in app_config: |
105 | 324 | self.logger.warn( | 294 | self._log_with_header( |
106 | 325 | "{} Application {} has no config for '{}', cannot determine if {} {}.".format( | 295 | "Application {} has no config for '{}', cannot determine if {} {}.".format( |
104 | 326 | model_header, | ||
107 | 327 | app_name, | 296 | app_name, |
108 | 328 | config_key, | 297 | config_key, |
109 | 329 | operator.repr, | 298 | operator.repr, |
110 | 330 | repr(check_value), | 299 | repr(check_value), |
112 | 331 | ) | 300 | ), |
113 | 301 | level=logging.WARN, | ||
114 | 332 | ) | 302 | ) |
115 | 333 | return False | 303 | return False |
116 | 334 | 304 | ||
117 | @@ -336,9 +306,8 @@ class Linter: | |||
118 | 336 | 306 | ||
119 | 337 | # Apply the check callable and handle the possible cases | 307 | # Apply the check callable and handle the possible cases |
120 | 338 | if operator.check(check_value, actual_value): | 308 | if operator.check(check_value, actual_value): |
124 | 339 | self.logger.debug( | 309 | self._log_with_header( |
125 | 340 | "{} Application {} has a valid config for '{}': {} ({} {})".format( | 310 | "Application {} has a valid config for '{}': {} ({} {})".format( |
123 | 341 | model_header, | ||
126 | 342 | app_name, | 311 | app_name, |
127 | 343 | config_key, | 312 | config_key, |
128 | 344 | repr(check_value), | 313 | repr(check_value), |
129 | @@ -373,26 +342,20 @@ class Linter: | |||
130 | 373 | """Check application against provided rules.""" | 342 | """Check application against provided rules.""" |
131 | 374 | rules = dict(rules) | 343 | rules = dict(rules) |
132 | 375 | for rule in rules: | 344 | for rule in rules: |
138 | 376 | self.logger.debug( | 345 | self._log_with_header("Checking {} for configuration {}".format(name, rule)) |
134 | 377 | "[{}] [{}/{}] Checking {} for configuration {}".format( | ||
135 | 378 | self.cloud_name, self.controller_name, self.model_name, name, rule | ||
136 | 379 | ) | ||
137 | 380 | ) | ||
139 | 381 | for check_op, check_value in rules[rule].items(): | 346 | for check_op, check_value in rules[rule].items(): |
140 | 382 | # check_op should be the operator name, e.g. (eq, neq, gte, isset) | 347 | # check_op should be the operator name, e.g. (eq, neq, gte, isset) |
141 | 383 | if check_op in VALID_CONFIG_CHECKS: | 348 | if check_op in VALID_CONFIG_CHECKS: |
142 | 384 | check_method = getattr(self, check_op) | 349 | check_method = getattr(self, check_op) |
143 | 385 | check_method(name, check_value, rule, config) | 350 | check_method(name, check_value, rule, config) |
144 | 386 | else: | 351 | else: |
150 | 387 | self.logger.warn( | 352 | self._log_with_header( |
151 | 388 | "[{}] [{}/{}] Application {} has unknown check operation for {}: {}.".format( | 353 | "Application {} has unknown check operation for {}: {}.".format( |
147 | 389 | self.cloud_name, | ||
148 | 390 | self.controller_name, | ||
149 | 391 | self.model_name, | ||
152 | 392 | name, | 354 | name, |
153 | 393 | rule, | 355 | rule, |
154 | 394 | check_op, | 356 | check_op, |
156 | 395 | ) | 357 | ), |
157 | 358 | level=logging.WARN, | ||
158 | 396 | ) | 359 | ) |
159 | 397 | 360 | ||
160 | 398 | def check_configuration(self, applications): | 361 | def check_configuration(self, applications): |
161 | @@ -401,13 +364,11 @@ class Linter: | |||
162 | 401 | # look for config rules for this application | 364 | # look for config rules for this application |
163 | 402 | lint_rules = [] | 365 | lint_rules = [] |
164 | 403 | if "charm" not in applications[application]: | 366 | if "charm" not in applications[application]: |
170 | 404 | self.logger.warn( | 367 | self._log_with_header( |
171 | 405 | "[{}] [{}/{}] Application {} has no charm.".format( | 368 | "Application {} has no charm.".format( |
167 | 406 | self.cloud_name, | ||
168 | 407 | self.controller_name, | ||
169 | 408 | self.model_name, | ||
172 | 409 | application, | 369 | application, |
174 | 410 | ) | 370 | ), |
175 | 371 | level=logging.WARN, | ||
176 | 411 | ) | 372 | ) |
177 | 412 | continue | 373 | continue |
178 | 413 | 374 | ||
179 | @@ -442,103 +403,51 @@ class Linter: | |||
180 | 442 | for required_sub in self.lint_rules["subordinates"]: | 403 | for required_sub in self.lint_rules["subordinates"]: |
181 | 443 | self.model.missing_subs.setdefault(required_sub, set()) | 404 | self.model.missing_subs.setdefault(required_sub, set()) |
182 | 444 | self.model.extraneous_subs.setdefault(required_sub, set()) | 405 | self.model.extraneous_subs.setdefault(required_sub, set()) |
188 | 445 | self.logger.debug( | 406 | self._log_with_header("Checking for sub {}".format(required_sub)) |
184 | 446 | "[{}] [{}/{}] Checking for sub {}".format( | ||
185 | 447 | self.cloud_name, self.controller_name, self.model_name, required_sub | ||
186 | 448 | ) | ||
187 | 449 | ) | ||
189 | 450 | where = self.lint_rules["subordinates"][required_sub]["where"] | 407 | where = self.lint_rules["subordinates"][required_sub]["where"] |
190 | 451 | for machine in self.model.subs_on_machines: | 408 | for machine in self.model.subs_on_machines: |
196 | 452 | self.logger.debug( | 409 | self._log_with_header("Checking on {}".format(machine)) |
192 | 453 | "[{}] [{}/{}] Checking on {}".format( | ||
193 | 454 | self.cloud_name, self.controller_name, self.model_name, machine | ||
194 | 455 | ) | ||
195 | 456 | ) | ||
197 | 457 | present_subs = self.model.subs_on_machines[machine] | 410 | present_subs = self.model.subs_on_machines[machine] |
198 | 458 | apps = self.model.apps_on_machines[machine] | 411 | apps = self.model.apps_on_machines[machine] |
199 | 459 | if where.startswith("on "): # only on specific apps | 412 | if where.startswith("on "): # only on specific apps |
200 | 460 | required_on = where[3:] | 413 | required_on = where[3:] |
208 | 461 | self.logger.debug( | 414 | self._log_with_header( |
209 | 462 | "[{}] [{}/{}] Requirement {} is = from...".format( | 415 | "Requirement {} is = from...".format(required_on) |
203 | 463 | self.cloud_name, | ||
204 | 464 | self.controller_name, | ||
205 | 465 | self.model_name, | ||
206 | 466 | required_on, | ||
207 | 467 | ) | ||
210 | 468 | ) | 416 | ) |
211 | 469 | if required_on not in apps: | 417 | if required_on not in apps: |
217 | 470 | self.logger.debug( | 418 | self._log_with_header("... NOT matched") |
213 | 471 | "[{}] [{}/{}] ... NOT matched".format( | ||
214 | 472 | self.cloud_name, self.controller_name, self.model_name | ||
215 | 473 | ) | ||
216 | 474 | ) | ||
218 | 475 | continue | 419 | continue |
220 | 476 | self.logger.debug("[{}] [{}/{}] ... matched") | 420 | self._log_with_header("... matched") |
221 | 477 | # TODO this needs to be not just one app, but a list | 421 | # TODO this needs to be not just one app, but a list |
222 | 478 | elif where.startswith("all except "): # not next to this app | 422 | elif where.startswith("all except "): # not next to this app |
228 | 479 | self.logger.debug( | 423 | self._log_with_header("requirement is != form...") |
224 | 480 | "[{}] [{}/{}] requirement is != form...".format( | ||
225 | 481 | self.cloud_name, self.controller_name, self.model_name | ||
226 | 482 | ) | ||
227 | 483 | ) | ||
229 | 484 | not_on = where[11:] | 424 | not_on = where[11:] |
230 | 485 | if not_on in apps: | 425 | if not_on in apps: |
236 | 486 | self.logger.debug( | 426 | self._log_with_header("... matched, not wanted on this host") |
232 | 487 | "[{}] [{}/{}] ... matched, not wanted on this host".format( | ||
233 | 488 | self.cloud_name, self.controller_name, self.model_name | ||
234 | 489 | ) | ||
235 | 490 | ) | ||
237 | 491 | continue | 427 | continue |
238 | 492 | elif where == "host only": | 428 | elif where == "host only": |
243 | 493 | self.logger.debug( | 429 | self._log_with_header( |
244 | 494 | "[{}] [{}/{}] requirement is 'host only' form....".format( | 430 | "requirement is 'host only' form...." |
241 | 495 | self.cloud_name, self.controller_name, self.model_name | ||
242 | 496 | ) | ||
245 | 497 | ) | 431 | ) |
246 | 498 | if is_container(machine): | 432 | if is_container(machine): |
252 | 499 | self.logger.debug( | 433 | self._log_with_header("... and we are a container, checking") |
248 | 500 | "[{}] [{}/{}] ... and we are a container, checking".format( | ||
249 | 501 | self.cloud_name, self.controller_name, self.model_name | ||
250 | 502 | ) | ||
251 | 503 | ) | ||
253 | 504 | # XXX check alternate names? | 434 | # XXX check alternate names? |
254 | 505 | if required_sub in present_subs: | 435 | if required_sub in present_subs: |
262 | 506 | self.logger.debug( | 436 | self._log_with_header("... found extraneous sub") |
256 | 507 | "[{}] [{}/{}] ... found extraneous sub".format( | ||
257 | 508 | self.cloud_name, | ||
258 | 509 | self.controller_name, | ||
259 | 510 | self.model_name, | ||
260 | 511 | ) | ||
261 | 512 | ) | ||
263 | 513 | for app in self.model.apps_on_machines[machine]: | 437 | for app in self.model.apps_on_machines[machine]: |
264 | 514 | self.model.extraneous_subs[required_sub].add(app) | 438 | self.model.extraneous_subs[required_sub].add(app) |
265 | 515 | continue | 439 | continue |
273 | 516 | self.logger.debug( | 440 | self._log_with_header("... and we are a host, will fallthrough") |
274 | 517 | "[{}] [{}/{}] ... and we are a host, will fallthrough".format( | 441 | |
268 | 518 | self.cloud_name, | ||
269 | 519 | self.controller_name, | ||
270 | 520 | self.model_name, | ||
271 | 521 | ) | ||
272 | 522 | ) | ||
275 | 523 | elif where == "all or nothing" and required_sub not in all_or_nothing: | 442 | elif where == "all or nothing" and required_sub not in all_or_nothing: |
282 | 524 | self.logger.debug( | 443 | self._log_with_header( |
283 | 525 | "[{}] [{}/{}] requirement is 'all or nothing' and was 'nothing'.".format( | 444 | "requirement is 'all or nothing' and was 'nothing'." |
278 | 526 | self.cloud_name, | ||
279 | 527 | self.controller_name, | ||
280 | 528 | self.model_name, | ||
281 | 529 | ) | ||
284 | 530 | ) | 445 | ) |
285 | 531 | continue | 446 | continue |
286 | 532 | # At this point we know we require the subordinate - we might just | 447 | # At this point we know we require the subordinate - we might just |
287 | 533 | # need to change the name we expect to see it as | 448 | # need to change the name we expect to see it as |
288 | 534 | elif where == "container aware": | 449 | elif where == "container aware": |
296 | 535 | self.logger.debug( | 450 | self._log_with_header("requirement is 'container aware'.") |
290 | 536 | "[{}] [{}/{}] requirement is 'container aware'.".format( | ||
291 | 537 | self.cloud_name, | ||
292 | 538 | self.controller_name, | ||
293 | 539 | self.model_name, | ||
294 | 540 | ) | ||
295 | 541 | ) | ||
297 | 542 | if is_container(machine): | 451 | if is_container(machine): |
298 | 543 | suffixes = self.lint_rules["subordinates"][required_sub][ | 452 | suffixes = self.lint_rules["subordinates"][required_sub][ |
299 | 544 | "container-suffixes" | 453 | "container-suffixes" |
300 | @@ -547,38 +456,17 @@ class Linter: | |||
301 | 547 | suffixes = self.lint_rules["subordinates"][required_sub][ | 456 | suffixes = self.lint_rules["subordinates"][required_sub][ |
302 | 548 | "host-suffixes" | 457 | "host-suffixes" |
303 | 549 | ] | 458 | ] |
312 | 550 | self.logger.debug( | 459 | self._log_with_header("-> suffixes == {}".format(suffixes)) |
305 | 551 | "[{}] [{}/{}] -> suffixes == {}".format( | ||
306 | 552 | self.cloud_name, | ||
307 | 553 | self.controller_name, | ||
308 | 554 | self.model_name, | ||
309 | 555 | suffixes, | ||
310 | 556 | ) | ||
311 | 557 | ) | ||
313 | 558 | exceptions = [] | 460 | exceptions = [] |
314 | 559 | if "exceptions" in self.lint_rules["subordinates"][required_sub]: | 461 | if "exceptions" in self.lint_rules["subordinates"][required_sub]: |
315 | 560 | exceptions = self.lint_rules["subordinates"][required_sub][ | 462 | exceptions = self.lint_rules["subordinates"][required_sub][ |
316 | 561 | "exceptions" | 463 | "exceptions" |
317 | 562 | ] | 464 | ] |
326 | 563 | self.logger.debug( | 465 | self._log_with_header("-> exceptions == {}".format(exceptions)) |
319 | 564 | "[{}] [{}/{}] -> exceptions == {}".format( | ||
320 | 565 | self.cloud_name, | ||
321 | 566 | self.controller_name, | ||
322 | 567 | self.model_name, | ||
323 | 568 | exceptions, | ||
324 | 569 | ) | ||
325 | 570 | ) | ||
327 | 571 | found = False | 466 | found = False |
328 | 572 | for suffix in suffixes: | 467 | for suffix in suffixes: |
329 | 573 | looking_for = "{}-{}".format(required_sub, suffix) | 468 | looking_for = "{}-{}".format(required_sub, suffix) |
338 | 574 | self.logger.debug( | 469 | self._log_with_header("-> Looking for {}".format(looking_for)) |
331 | 575 | "[{}] [{}/{}] -> Looking for {}".format( | ||
332 | 576 | self.cloud_name, | ||
333 | 577 | self.controller_name, | ||
334 | 578 | self.model_name, | ||
335 | 579 | looking_for, | ||
336 | 580 | ) | ||
337 | 581 | ) | ||
339 | 582 | if looking_for in present_subs: | 470 | if looking_for in present_subs: |
340 | 583 | self.logger.debug("-> FOUND!!!") | 471 | self.logger.debug("-> FOUND!!!") |
341 | 584 | self.cloud_name, | 472 | self.cloud_name, |
342 | @@ -588,44 +476,24 @@ class Linter: | |||
343 | 588 | if not found: | 476 | if not found: |
344 | 589 | for sub in present_subs: | 477 | for sub in present_subs: |
345 | 590 | if self.model.app_to_charm[sub] == required_sub: | 478 | if self.model.app_to_charm[sub] == required_sub: |
353 | 591 | self.logger.debug( | 479 | self._log_with_header( |
354 | 592 | "[{}] [{}/{}] Winner winner, chicken dinner! 🍗 {}".format( | 480 | "Winner winner, chicken dinner! 🍗 {}".format(sub) |
348 | 593 | self.cloud_name, | ||
349 | 594 | self.controller_name, | ||
350 | 595 | self.model_name, | ||
351 | 596 | sub, | ||
352 | 597 | ) | ||
355 | 598 | ) | 481 | ) |
356 | 599 | found = True | 482 | found = True |
357 | 600 | if not found: | 483 | if not found: |
358 | 601 | for exception in exceptions: | 484 | for exception in exceptions: |
359 | 602 | if exception in apps: | 485 | if exception in apps: |
366 | 603 | self.logger.debug( | 486 | self._log_with_header( |
367 | 604 | "[{}] [{}/{}]-> continuing as found exception: {}".format( | 487 | "continuing as found exception: {}".format( |
368 | 605 | self.cloud_name, | 488 | exception |
363 | 606 | self.controller_name, | ||
364 | 607 | self.model_name, | ||
365 | 608 | exception, | ||
369 | 609 | ) | 489 | ) |
370 | 610 | ) | 490 | ) |
371 | 611 | found = True | 491 | found = True |
372 | 612 | if not found: | 492 | if not found: |
380 | 613 | self.logger.debug( | 493 | self._log_with_header("-> NOT FOUND") |
374 | 614 | "[{}] [{}/{}] -> NOT FOUND".format( | ||
375 | 615 | self.cloud_name, | ||
376 | 616 | self.controller_name, | ||
377 | 617 | self.model_name, | ||
378 | 618 | ) | ||
379 | 619 | ) | ||
381 | 620 | for app in self.model.apps_on_machines[machine]: | 494 | for app in self.model.apps_on_machines[machine]: |
382 | 621 | self.model.missing_subs[required_sub].add(app) | 495 | self.model.missing_subs[required_sub].add(app) |
390 | 622 | self.logger.debug( | 496 | self._log_with_header("-> continue-ing back out...") |
384 | 623 | "[{}] [{}/{}] -> continue-ing back out...".format( | ||
385 | 624 | self.cloud_name, | ||
386 | 625 | self.controller_name, | ||
387 | 626 | self.model_name, | ||
388 | 627 | ) | ||
389 | 628 | ) | ||
391 | 629 | continue | 497 | continue |
392 | 630 | elif where not in ["all", "all or nothing"]: | 498 | elif where not in ["all", "all or nothing"]: |
393 | 631 | self.logger.fubar( | 499 | self.logger.fubar( |
394 | @@ -637,30 +505,15 @@ class Linter: | |||
395 | 637 | required_sub, | 505 | required_sub, |
396 | 638 | ) | 506 | ) |
397 | 639 | ) | 507 | ) |
405 | 640 | self.logger.debug( | 508 | self._log_with_header("requirement is 'all' OR we fell through.") |
399 | 641 | "[{}] [{}/{}] requirement is 'all' OR we fell through.".format( | ||
400 | 642 | self.cloud_name, | ||
401 | 643 | self.controller_name, | ||
402 | 644 | self.model_name, | ||
403 | 645 | ) | ||
404 | 646 | ) | ||
406 | 647 | if required_sub not in present_subs: | 509 | if required_sub not in present_subs: |
407 | 648 | for sub in present_subs: | 510 | for sub in present_subs: |
408 | 649 | if self.model.app_to_charm[sub] == required_sub: | 511 | if self.model.app_to_charm[sub] == required_sub: |
410 | 650 | self.logger.debug( | 512 | self._log_with_header( |
411 | 651 | "Winner winner, chicken dinner! 🍗 {}".format(sub) | 513 | "Winner winner, chicken dinner! 🍗 {}".format(sub) |
412 | 652 | ) | 514 | ) |
413 | 653 | self.cloud_name, | ||
414 | 654 | self.controller_name, | ||
415 | 655 | self.model_name, | ||
416 | 656 | continue | 515 | continue |
424 | 657 | self.logger.debug( | 516 | self._log_with_header("not found.") |
418 | 658 | "[{}] [{}/{}] not found.".format( | ||
419 | 659 | self.cloud_name, | ||
420 | 660 | self.controller_name, | ||
421 | 661 | self.model_name, | ||
422 | 662 | ) | ||
423 | 663 | ) | ||
425 | 664 | for app in self.model.apps_on_machines[machine]: | 517 | for app in self.model.apps_on_machines[machine]: |
426 | 665 | self.model.missing_subs[required_sub].add(app) | 518 | self.model.missing_subs[required_sub].add(app) |
427 | 666 | 519 | ||
428 | @@ -904,10 +757,9 @@ class Linter: | |||
429 | 904 | """Map machines in the model to their availability zone.""" | 757 | """Map machines in the model to their availability zone.""" |
430 | 905 | for machine in machines: | 758 | for machine in machines: |
431 | 906 | if "hardware" not in machines[machine]: | 759 | if "hardware" not in machines[machine]: |
436 | 907 | self.logger.warn( | 760 | self._log_with_header( |
437 | 908 | "[{}] [{}/{}] Machine {} has no hardware info; skipping.".format( | 761 | "Machine {} has no hardware info; skipping.".format(machine), |
438 | 909 | self.cloud_name, self.controller_name, self.model_name, machine | 762 | level=logging.WARN, |
435 | 910 | ) | ||
439 | 911 | ) | 763 | ) |
440 | 912 | continue | 764 | continue |
441 | 913 | 765 | ||
442 | @@ -920,10 +772,11 @@ class Linter: | |||
443 | 920 | self.model.machines_to_az[machine] = az | 772 | self.model.machines_to_az[machine] = az |
444 | 921 | break | 773 | break |
445 | 922 | if not found_az: | 774 | if not found_az: |
450 | 923 | self.logger.warn( | 775 | self._log_with_header( |
451 | 924 | "[{}] [{}/{}] Machine {} has no availability-zone info in hardware field; skipping.".format( | 776 | "Machine {} has no availability-zone info in hardware field; skipping.".format( |
452 | 925 | self.cloud_name, self.controller_name, self.model_name, machine | 777 | machine |
453 | 926 | ) | 778 | ), |
454 | 779 | level=logging.WARN, | ||
455 | 927 | ) | 780 | ) |
456 | 928 | 781 | ||
457 | 929 | def check_status(self, what, status, expected): | 782 | def check_status(self, what, status, expected): |
458 | @@ -987,19 +840,16 @@ class Linter: | |||
459 | 987 | expected=juju_expected, | 840 | expected=juju_expected, |
460 | 988 | ) | 841 | ) |
461 | 989 | else: | 842 | else: |
466 | 990 | self.logger.warn( | 843 | self._log_with_header( |
467 | 991 | "[{}] [{}/{}] Could not determine Juju status for {}.".format( | 844 | "Could not determine Juju status for {}.".format(name), |
468 | 992 | self.cloud_name, self.controller_name, self.model_name, name | 845 | level=logging.WARN, |
465 | 993 | ) | ||
469 | 994 | ) | 846 | ) |
470 | 995 | else: | 847 | else: |
476 | 996 | self.logger.warn( | 848 | self._log_with_header( |
477 | 997 | "[{}] [{}/{}] Could not determine appropriate status key for {}.".format( | 849 | "Could not determine appropriate status key for {}.".format( |
473 | 998 | self.cloud_name, | ||
474 | 999 | self.controller_name, | ||
475 | 1000 | self.model_name, | ||
478 | 1001 | name, | 850 | name, |
480 | 1002 | ) | 851 | ), |
481 | 852 | level=logging.WARN, | ||
482 | 1003 | ) | 853 | ) |
483 | 1004 | 854 | ||
484 | 1005 | def check_statuses(self, juju_status, applications): | 855 | def check_statuses(self, juju_status, applications): |
485 | @@ -1073,14 +923,12 @@ class Linter: | |||
486 | 1073 | machine = applications[app_name]["units"][unit]["machine"] | 923 | machine = applications[app_name]["units"][unit]["machine"] |
487 | 1074 | machine = machine.split("/")[0] | 924 | machine = machine.split("/")[0] |
488 | 1075 | if machine not in self.model.machines_to_az: | 925 | if machine not in self.model.machines_to_az: |
494 | 1076 | self.logger.error( | 926 | self._log_with_header( |
495 | 1077 | "[{}] [{}/{}] {}: Can't find machine {} in machine to AZ mapping data".format( | 927 | "{}: Can't find machine {} in machine to AZ mapping data".format( |
491 | 1078 | self.cloud_name, | ||
492 | 1079 | self.controller_name, | ||
493 | 1080 | self.model_name, | ||
496 | 1081 | app_name, | 928 | app_name, |
497 | 1082 | machine, | 929 | machine, |
499 | 1083 | ) | 930 | ), |
500 | 931 | level=logging.ERROR, | ||
501 | 1084 | ) | 932 | ) |
502 | 1085 | continue | 933 | continue |
503 | 1086 | az_counter[self.model.machines_to_az[machine]] += 1 | 934 | az_counter[self.model.machines_to_az[machine]] += 1 |
504 | @@ -1133,25 +981,14 @@ class Linter: | |||
505 | 1133 | self.check_azs(parsed_yaml[applications]) | 981 | self.check_azs(parsed_yaml[applications]) |
506 | 1134 | self.check_statuses(parsed_yaml, applications) | 982 | self.check_statuses(parsed_yaml, applications) |
507 | 1135 | else: | 983 | else: |
517 | 1136 | self.logger.warn( | 984 | self._log_with_header( |
518 | 1137 | ( | 985 | "Relations data found; assuming a bundle and skipping AZ and status checks." |
510 | 1138 | "[{}] [{}/{}] Relations data found; assuming a bundle and " | ||
511 | 1139 | "skipping AZ and status checks." | ||
512 | 1140 | ).format( | ||
513 | 1141 | self.cloud_name, | ||
514 | 1142 | self.model_name, | ||
515 | 1143 | self.controller_name, | ||
516 | 1144 | ) | ||
519 | 1145 | ) | 986 | ) |
520 | 1146 | 987 | ||
521 | 1147 | self.results() | 988 | self.results() |
522 | 1148 | else: | 989 | else: |
529 | 1149 | self.logger.warn( | 990 | self._log_with_header( |
530 | 1150 | "[{}] [{}/{}] Model contains no applications, skipping.".format( | 991 | "Model contains no applications, skipping.", level=logging.WARN |
525 | 1151 | self.cloud_name, | ||
526 | 1152 | self.controller_name, | ||
527 | 1153 | self.model_name, | ||
528 | 1154 | ) | ||
531 | 1155 | ) | 992 | ) |
532 | 1156 | 993 | ||
533 | 1157 | def collect(self, error): | 994 | def collect(self, error): |
534 | @@ -1160,11 +997,7 @@ class Linter: | |||
535 | 1160 | 997 | ||
536 | 1161 | def handle_error(self, error): | 998 | def handle_error(self, error): |
537 | 1162 | """Collect an error and add it to the collector.""" | 999 | """Collect an error and add it to the collector.""" |
543 | 1163 | self.logger.error( | 1000 | self._log_with_header(error["message"], level=logging.ERROR) |
539 | 1164 | "[{}] [{}/{}] {}.".format( | ||
540 | 1165 | self.cloud_name, self.controller_name, self.model_name, error["message"] | ||
541 | 1166 | ) | ||
542 | 1167 | ) | ||
544 | 1168 | if self.collect_errors: | 1001 | if self.collect_errors: |
545 | 1169 | self.collect(error) | 1002 | self.collect(error) |
546 | 1170 | 1003 | ||
547 | @@ -1184,7 +1017,9 @@ class Linter: | |||
548 | 1184 | try: | 1017 | try: |
549 | 1185 | _, rel_path = line.split() | 1018 | _, rel_path = line.split() |
550 | 1186 | except ValueError: | 1019 | except ValueError: |
552 | 1187 | self.logger.warn("invalid include in rules, ignored: '{}'".format(line)) | 1020 | self.logger.warn( |
553 | 1021 | "invalid include in rules, ignored: '{}'".format(line) | ||
554 | 1022 | ) | ||
555 | 1188 | continue | 1023 | continue |
556 | 1189 | 1024 | ||
557 | 1190 | include_path = os.path.join(os.path.dirname(self.filename), rel_path) | 1025 | include_path = os.path.join(os.path.dirname(self.filename), rel_path) |
558 | @@ -1196,3 +1031,12 @@ class Linter: | |||
559 | 1196 | collector.append(line) | 1031 | collector.append(line) |
560 | 1197 | 1032 | ||
561 | 1198 | return yaml.safe_load("\n".join(collector)) | 1033 | return yaml.safe_load("\n".join(collector)) |
562 | 1034 | |||
563 | 1035 | def _log_with_header(self, msg, level=logging.DEBUG): | ||
564 | 1036 | """Log a message with the cloud/controller/model header.""" | ||
565 | 1037 | self.logger.log( | ||
566 | 1038 | "[{}] [{}/{}] {}".format( | ||
567 | 1039 | self.cloud_name, self.controller_name, self.model_name, msg | ||
568 | 1040 | ), | ||
569 | 1041 | level=level, | ||
570 | 1042 | ) | ||
571 | diff --git a/jujulint/logging.py b/jujulint/logging.py | |||
572 | index 47eaa88..e3c42f8 100644 | |||
573 | --- a/jujulint/logging.py | |||
574 | +++ b/jujulint/logging.py | |||
575 | @@ -105,3 +105,7 @@ class Logger: | |||
576 | 105 | def error(self, message): | 105 | def error(self, message): |
577 | 106 | """Log a message with warn loglevel.""" | 106 | """Log a message with warn loglevel.""" |
578 | 107 | self.logger.error(message) | 107 | self.logger.error(message) |
579 | 108 | |||
580 | 109 | def log(self, message, level=logging.DEBUG): | ||
581 | 110 | """Log a message with arbitrary loglevel.""" | ||
582 | 111 | self.logger.log(level, message) |
This merge proposal is being monitored by mergebot. Change the status to Approved to merge.