john.levon@xxxxxxx wrote: > # HG changeset patch > # User john.levon@xxxxxxx > # Date 1228851891 28800 > # Node ID 29d8886362e2993eaf26cf8d4e948b2de4b8d9ec > # Parent 3a48e2d3594b3e7e1d24147f3325ab6024557504 > Port utility functions to Solaris > Hmm, 90% of this patch is a huge mechanical change which has nothing to do with this commit subject. I think it's better to separate the two changes. > Create _util.py for private details of the implementation, along with a > shim for back compatibility. > > Port the utilities to Solaris. > > Signed-off-by: John Levon <john.levon@xxxxxxx> > > diff --git a/virtinst/CloneManager.py b/virtinst/CloneManager.py > --- a/virtinst/CloneManager.py > +++ b/virtinst/CloneManager.py > @@ -23,7 +23,7 @@ import libxml2 > import libxml2 > import logging > import urlgrabber.progress as progress > -import util > +import _util Rather than s/util/_util/ everywhere, I'd prefer: import _util as util Lot less churn, and the only potential downside is confusion browsing the source, but I don't think it's a problem. <snip> > diff --git a/virtinst/util.py b/virtinst/util.py > --- a/virtinst/util.py > +++ b/virtinst/util.py > @@ -1,5 +1,3 @@ > -# > -# Utility functions used for guest installation > # > # Copyright 2006 Red Hat, Inc. > # Jeremy Katz <katzj@xxxxxxxxxx> > @@ -19,107 +17,60 @@ > # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, > # MA 02110-1301 USA. > > +# > +# WARNING: this file sadly forms part of the public API. Do not add > +# anything to this file! The file for internal utility functions is > +# _util.py > +# > + > import platform > -import random > -import os.path > -import re > -import libxml2 > -import logging > -from sys import stderr > +import os > > -import libvirt > -from virtinst import _virtinst as _ > -from virtinst import CapabilitiesParser > +from virtinst import _util > > > KEYBOARD_DIR = "/etc/sysconfig/keyboard" > XORG_CONF = "/etc/X11/xorg.conf" > > -def default_route(): > - route_file = "/proc/net/route" > - d = file(route_file) > +default_route = _util.default_route > +default_bridge = _util.default_bridge > +default_network = _util.default_network > +default_connection = _util.default_connection > +get_cpu_flags = _util.get_cpu_flags > +is_pae_capable = _util.is_pae_capable > +is_blktap_capable = _util.is_blktap_capable > +get_default_arch = _util.get_default_arch > +randomMAC = _util.randomMAC > +randomUUID = _util.randomUUID > +uuidToString = _util.uuidToString > +uuidFromString = _util.uuidFromString > +get_host_network_devices = _util.get_host_network_devices > +get_max_vcpus = _util.get_max_vcpus > +get_phy_cpus = _util.get_phy_cpus > +xml_escape = _util.xml_escape > +compareMAC = _util.compareMAC > +default_keymap = _util.default_keymap > +pygrub_path = _util.pygrub_path > +uri_split = _util.uri_split > +is_uri_remote = _util.is_uri_remote > +get_uri_hostname = _util.get_uri_hostname > +get_uri_transport = _util.get_uri_transport > +get_uri_driver = _util.get_uri_driver > +is_storage_capable = _util.is_storage_capable > +get_xml_path = _util.get_xml_path > +lookup_pool_by_path = _util.lookup_pool_by_path > +privileged_user = _util.privileged_user > Hmm, this isn't exactly how I saw it happening. I figured we would have _util be for nonpublic functions only: we can change or remove anything in that file at will. The original 'util' though needs to maintain api compat for its functions, even if they are deprecated and left to rot. That would mean 'util' would stay the same. '_util' would have whatever new functions you are adding, and the equivalent of the above block of code. Anyone needing a utility function for just the library will add it to _util, and if we ever want to make a private utility public, just move it to 'util'. Am I missing some benefit of the above approach? Or are we just on different pages? (My intention really isn't to be a pest :) ) Thanks, Cole _______________________________________________ et-mgmt-tools mailing list et-mgmt-tools@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/et-mgmt-tools
