dns._EDNSMessage should use meaningful constructor argument names and attributes

[Richard Wall]

dns._EDNSMessage has been written for compatibility with dns.Message. Which means that it inherits all the horrible abbreviated constructor argument and attribute names.

Maybe that was a mistake. Instead lets try and give dns._EDNSMessage the ideal interface and then provide adaptors for dns.Message if necessary.

[Richard Wall]

(In [40926]) Branching to 'better-edns-message-arguments-6862'

[Richard Wall]

Ready for review in log:branches/better-edns-message-arguments-6862

1. I updated the _EDNSMessage constructor arguments
2. ...and added an alternative constructor to dns.Message which I've provisionally called sensibleConstructor.
3. ...and added properties to dns.Message which allow the full attribute names to be queried.
4. I experimented with using twisted.python.constants - it soon got complicated when I tried to share the tests with dns.Message.
   1. And FlagConstants didn't seem appropriate because the Message flags don't need any values.

I'd like some feedback on this approach. Ultimately, I'd like to deprecate the old cryptic attributes and constructor arguments on dns.Message, so perhaps it would be better to make properties for the old attribute names which issue deprecation warnings. But what's the best way to deprecate the dns.Message constructor argument names?

Build results:

• ​https://buildbot.twistedmatrix.com/boxes-supported?branch=/branches/better-edns-message-arguments-6862

[Richard Wall]

(In [41140]) Branching to 'better-edns-message-arguments-6862-2'

[Richard Wall]

I've redone this branch in log:better-edns-message-arguments-6862-2

• This time without making any changes to dns.Message
• So I've had to do some further adaptation of the tests; converting constructor argument names before passing them to dns.Message
• Ultimately, I want dns.Message to have the same meaningful attribute names and constructor arguments, so once this is merged, I'll create a branch to add a deprecate the use the old Message names.
• or deprecate Message and provide a new alternative (StandardMessage) with good names

Build Results:

• ​https://buildbot.twistedmatrix.com/boxes-supported?branch=/branches/better-edns-message-arguments-6862-2

[Richard Wall]

Introduce a new "StandardMessage" class with meaningful constructor args and attribute names. Deprecate dns.Message.

[Richard Wall]

In attachment:deprecate-message-6862.patch​ (against better-edns-message-arguments-6862-2)

• Hacked together a new "StandardMessage" class which has meaningful constructor arguments and attribute names.
• It re-uses dns.Message for it's encoding and decoding (until such a time as dns.Message can be removed)
• Made dns.Message private and introduced a deprecated subclass
• Refactored the tests so that constructor tests can be shared between the three message classes.
• Made _EDNSMessage.fromStr a class method -- which is much neater and leads to shorter tests.
• Introduced a LegacyMessage adaptor so that the inplace version of fromStr in dns.Message can be tested using the same tests as _EDNSMessage and StandardMessage.

There's one failing test -- caused by my commenting out "self.maxSize = 0" in Message.decode. I commented it out, because it was causing Message comparisons to fail between newly instantiated Messages and decoded messages.

Need to investigate, I suspect a bug.

So anyway, this patch gives an idea about the direction I'm heading. I'm interested to know if it's the right direction.

If so, I can split the patch into separate tickets for

• Introduction of "StandardMessage"
• Replace use of Message with StandardMessage within twisted code (possibly one ticket per module)
• Deprecate dns.Message
• Remove dns.Message

[Glyph]

So, "I definitely like this direction". Much better docs, I like the less mutation-oriented approach, etc. Are you looking for more guidance than that? My one caveat here would be that StandardMessage is a bit wordy, and it's then not clear what the distinction with Message is until the deprecation is fully done.

I might also make it a bit easier to convert back and forth so that legacy code that is processing Message instances can start moving.