Skip to content
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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Pat-pGuo
Copy link

@Pat-pGuo Pat-pGuo commented Nov 29, 2024

For review:

  • implement leaf datatypes
  • implement structural datatypes
  • make dictionary.Attribute.encode() and decode() datatype agnostic
  • make packet.DecodePacket() datatype agnostic
  • existing unittests fixes

Work in progress:

@Pat-pGuo Pat-pGuo force-pushed the feature/dynamic-datatype-loader branch from 9eafd73 to 8a8a4d9 Compare December 2, 2024 21:11
':'.join(map('{0:02x}'.format, struct.unpack('!6B', raw)))

def print(self, attribute, decoded):
return decoded

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

Copy link
Author

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()

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?

Copy link
Author

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.

@Pat-pGuo Pat-pGuo force-pushed the feature/dynamic-datatype-loader branch 2 times, most recently from 03e85ad to 9a1c6b5 Compare December 6, 2024 18:10
@Pat-pGuo Pat-pGuo force-pushed the feature/dynamic-datatype-loader branch from 9a1c6b5 to 60349c0 Compare December 6, 2024 18:35
@Pat-pGuo Pat-pGuo marked this pull request as ready for review December 6, 2024 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants