BIND 10 #1866: isc.dns constants like RRType.A() should be constants, not functions

BIND 10 Development do-not-reply at isc.org
Mon Feb 4 22:25:22 UTC 2013


#1866: isc.dns constants like RRType.A() should be constants, not functions
-------------------------------------+-------------------------------------
            Reporter:  jinmei        |                        Owner:
                Type:  defect        |  vorner
            Priority:  medium        |                       Status:
           Component:  libdns++      |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-20130205
         Sub-Project:  DNS           |                   Resolution:
Estimated Difficulty:  5             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Thanks for the review.

 Replying to [comment:14 vorner]:

 > First, would it make sense to put the script to convert into the
 repository, let's say to the tools directory? In case some user already
 used our libraries, it might be useful for them. We should, of course,
 make it clear it is not supported in any way.

 I don't necessarily oppose to that, but at this time I'd rather avoid
 including mostly-unused file.  Realistically, I suspect there are yet
 very few people using this Python library for development other than
 ourselves.

 > I think the convention how to keep empty git directories is to place a
 `.keep` file there, not `gitignore`. The `.gitignore` has special meaning
 for git and it feels a bit like an abuse.

 Okay, but I've actually removed the directory with the dot- file
 based on your other comment below, so this point is now moot.

 > The tests fail for me:
 > {{{
 > Running test: session_tests.py
 > ...F..........F..........................
 > ======================================================================
 > FAIL: test_convert_rrset_class (__main__.SessionModuleTests)
 > }}}

 As I responded separately, I couldn't reproduce it, and I suspect it
 was because your build environment is incomplete.

 > It is not code from the branch, but this regexp seems to be wrong
 (missing a dot after the backslash). It is in the generator script:
 > {{{#!python
 > elif re.search('\cc$', file):
 > }}}

 Ah, right, fixed.

 > Why do you generate the code into a variable and accumulate it across
 all the RRtypes and then dump it after each new one? This seems like a
 quadratic run time and it looks unnecessary. Would it make sense to output
 it just once at the end?

 I guess you (at least) meant generate_typeclasscode(), where we built
 the auto-generated code by extending a string object.  I'm not sure if
 the internal implementation is that inefficient, and even if so, if it
 matters in this usage.  But I revised the code to what I guess you
 would have envisioned.

 > Would it be more lightweight and easier to read if the list of „fake“
 types (the ones that don't exist or we don't have them implemented yet)
 would be by a single file with a list instead of many effectively empty
 files?

 Hmm, I once considered that approach and at that time it seemed to
 require heavier updates to the generator script.  But on looking at it
 again it seems to be quite straightforward.  So I adopted the idea
 (but instead of introducing a separate file, simply listing them in
 the generator script), removing the placeholder files.

-- 
Ticket URL: <http://bind10.isc.org/ticket/1866#comment:17>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development


More information about the bind10-tickets mailing list