Attention is currently required from: neels.

View Change

3 comments:

• File pySim/esim/saip/param_source.py:

  • Patch Set #6, Line 121: val = random.randbytes(self.num_digits // 2) # TODO secure random source?

    Done

    Ah ok, I understand. The caller can digest hexstrings and bytearrays. I wasn not aware of this. To mee it looked a bit off that all get_next methods return hexstrings except this one.

    However, if it is ok to return either a hexstring or bytes. Then you could also add a comment + type annotation to the get_next method in ParamSource.

• File pySim/esim/saip/param_source.py:

  • Patch Set #8, Line 116:

    I wonder what happens when input_str is something like this: "12..345". Then num_digits is 2 but the last_value actually has 3 digits. As you assign self.num_digits I guess that you need this value for something later. I would recommend to check back if it is ok when self.num_digits is not equal to the actual number of digits in self.last_value.

  • Patch Set #8, Line 131: val = random.randint(*self.val_first_last) # TODO secure random source?

    The line where you assign self.val_first_last in DecimalRangeSource has been removed (this also seems to be the reason why pylint fails). Maybe you can do:

    random.randint(self.first_value, self.last_value)

To view, visit change 41845. To unsubscribe, or for help writing mail filters, visit settings.