BIND 10 #2172: Add support for OS X in b10-showtech
BIND 10 Development
do-not-reply at isc.org
Wed Aug 8 00:03:05 UTC 2012
#2172: Add support for OS X in b10-showtech
-------------------------------------+-------------------------------------
Reporter: jelte | Owner: jinmei
Type: | Status: reviewing
defect | Milestone:
Priority: | Sprint-20120821
medium | Resolution:
Component: | Sensitive: 0
Unclassified | Sub-Project: DNS
Keywords: | Estimated Difficulty: 0
Defect Severity: N/A | Total Hours: 0
Feature Depending on Ticket: |
Add Hours to Ticket: 3 |
Internal?: 0 |
-------------------------------------+-------------------------------------
Comment (by jinmei):
'''sysinfo.py'''
- (although not specific to this branch) I don't think it a good
practice to have Linux specific notions like mem_cached,
mem_buffers, or get_platform_distro() at the top level. It seems to
be a typical example of the "the world is Linux" mindset. IMO we
should either introduce a higher abstraction for these that can
apply more system independently or move them to Linux specific code.
- The implementation seems to share much of the code with the FreeBSD
version. I'd unify them as much as possible based on the DRY
principle.
- at least on my MBP kern.smp.active isn't available:
{{{
% sysctl kern.smp.active
second level name smp in kern.smp.active is invalid
}}}
and I don't even know if it's possible to activate/deactivate an SMP
support on MacOS X.
- I don't think it a good idea to have a hardcoded page_size
default. If it's not known (which should be very rare or indicate
we need to update the parser script), IMO we should also leave
parameters that depend on it unknown.
- Likewise (and although it's not specific to the MacOS version), I'd
generally avoid using magic numbers like "-1" to represent
"unknown", "unavailable", etc, because magic numbers make the intent
of the code more vague and can often lead to unexpected bugs (e.g.)
as a result of automatic conversion. Python has a perfection
representation for this type of concepts: None. I'd have
get_mem_free, etc return None when the value is unknown and convert
it to corresponding string at the main program, or we could simply
have get_xxx return the corresponding string if we only use them for
printing purposes.
- The `_mem_free` value doesn't match the corresponding "Free" size
shown in Activity Monitor. It looks like we should use the sum of
'Pages free' and 'Pages speculative' instead of only the former.
See also:
http://www.opensource.apple.com/source/xnu/xnu-1456.1.26/osfmk/mach/vm_statistics.h
- hw.physmem doesn't match the actual available memory, at least on my
MBP. hw.memsize seems to be the one that should be used. See:
http://stackoverflow.com/questions/583736/determine-physical-mem-size-
programmatically-on-osx
'''sysinfo_test.py'''
- `_my_osx_os_sysconf`: what's the magic number of 91? Arbitrary
chosen? As a magic number hater, I'd rather define a constant
variable with commenting the intent and use the variable throughout
the file. Also, using the same constants for different systems is
probably not a good idea anyway as we wouldn't be able to catch some
type of regression.
- `_my_osx_subprocess_check_output`: this generally seems to be the
same as that for FreeBSD. If we unify the main code (see above),
it's probably better to unify the test case, too. Or, if we want to
separate the two test cases (which is not necessarily a bad idea),
it's probably better to use different faked constants.
- `_my_osx_subprocess_check_output`: maybe related to the previous
point, 'daemon' doesn't seem to be very compatible with OS X.
{{{#!python
if command == ['hostname']:
return b'daemon.example.com\n'
}}}
- test_sysinfo_osx: I've fixed a trivial typo in a comment. And, this
seems to suggest we might rather unify these test_xxx's and switch
from it depending on the system type, rather than repeating the same
logic for every case.
- test_sysinfo_osx: not really specific to this branch, but it'd be
safer to restore the original functions in tearDown().
--
Ticket URL: <http://bind10.isc.org/ticket/2172#comment:5>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development
More information about the bind10-tickets
mailing list