Opened 7 years ago

Closed 7 years ago

#2119 closed defect (fixed)

cfgmgr loading of config check plugins

Reported by: jelte Owned by: jelte
Priority: medium Milestone: Sprint-20120731
Component: Unclassified Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DNS Feature Depending on Ticket:
Estimated Difficulty: 3 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

The configmanager has a nifty feature for loading configuration check plugins; However, it does so by adding the plugin path to the python path, and then using import.

Even though it puts the path at the start, I ran into a problem on one of my systems where it failed to load one due to a naming conflict (the up and coming datasrc.py), I think because a module with the same name was already loaded. This can be avoided by careful naming, but I think there is a better solution; use the imp module and its load_source() method (we need to make sure it is not reloaded, taken from cache, or overwrite any existing module)

Subtickets

Attachments (1)

bind10_cfgmgr_plugin_imports.diff (1.7 KB) - added by jelte 7 years ago.

Download all attachments as: .zip

Change History (8)

Changed 7 years ago by jelte

comment:1 Changed 7 years ago by jelte

Attached a diff as an example of an approach

comment:2 Changed 7 years ago by jelte

  • Milestone changed from New Tasks to Next-Sprint-Proposed

comment:3 Changed 7 years ago by jinmei

Not sure if it's related, but I'd like to revisit the possibly
unnecessary import at the package level in
src/lib/python/isc/__init__.py:

import isc.cc
import isc.config
import isc.datasrc

comment:4 Changed 7 years ago by jelte

  • Milestone changed from Next-Sprint-Proposed to Sprint-20120731

comment:5 Changed 7 years ago by vorner

  • Owner set to vorner
  • Status changed from new to reviewing

There's code already, let's just review it and be done with it.

comment:6 Changed 7 years ago by vorner

  • Owner changed from vorner to jelte

The patch seems reasonable and after renaming a plugin to datasrc.py, it seems to work. So I think it should be applied.

Regarding the __init__.py thing from jinmei, I think it is a good idea, but it should go to a separate ticket. Maybe a candidate for the cleanup sprint?

comment:7 Changed 7 years ago by jelte

  • Resolution set to fixed
  • Status changed from reviewing to closed

Ack, definitely a separate issue (and possibly not as trivial as it seems, given the comment there)

merged this for now, closing ticket, created another ticket for jinmei's comment (#2145)

Note: See TracTickets for help on using tickets.