-
Notifications
You must be signed in to change notification settings - Fork 187
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIP: use data driven approach when interacting with datatypes #205
base: master
Are you sure you want to change the base?
WIP: use data driven approach when interacting with datatypes #205
Conversation
9eafd73
to
8a8a4d9
Compare
':'.join(map('{0:02x}'.format, struct.unpack('!6B', raw))) | ||
|
||
def print(self, attribute, decoded): | ||
return decoded |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand what the "print" functions are supposed to do here.
The requirement for print
is "print this value to an ASCII string". The class also needs a related parse
routine. which takes an ASCII string, and returns a python structure / dict containing the attribute
This functionality lets us do the following tests:
decode HEX -> python stuff
print python stuff - ASCII
Human reads the ASCII and manually checks it agains the HEX to be sure it's OK.
We can then do
parse ASCII -> python stuff
encode python stuff -> HEX
which should then result in the same HEX output as above.
Once the unit tests have been verified manually (or copied from FreeRADIUS), the tests can be run in CI. The tests then ensure that:
a) the decoder can handle the packets correctly
b) the ASCII output doesn't change
c) the parser can read ASCII output
d) the encoder can encode things correctly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So yes, the print
function is returning an ASCII string. It just so happens that Pyrad has been storing these datatypes as strings, thus I simply return the value (which is an ASCII string).
cls.integer64 = Integer64() | ||
cls.ipaddr = Ipaddr() | ||
cls.ipv6addr = Ipv6addr() | ||
cls.ipv6prefix = Ipv6prefix() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there an IPv4 prefix type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As per discussions with @arr2036, that's planned for the next PR (which will contain all of the datatypes not currently in Pyrad). This PR is for restructuring existing datatypes.
03e85ad
to
9a1c6b5
Compare
9a1c6b5
to
60349c0
Compare
For review:
dictionary.Attribute.encode()
anddecode()
datatype agnosticpacket.DecodePacket()
datatype agnosticWork in progress: