Should we additionally replace all existing direct ensure_x() calls in this patch as well?

Also, maybe get_attr_vals_str() as a name is just as fine? Since I guess we're not going to use other encodings than UTF-8. Or are we?

I started to replace a few, but we can't just blindly replace all the calls. Only calls where we get something from the objects into a certain type. We should certainly aim to achieve this, but there are likely test cases around the place that still use the ensure_() funcs.

We may have other encodings beside utf8, I don't know today. So I think being clear that this is utf8 is important. ensure_str should have been ensure_utf8, but I can't easily undo past-William's mistake now :)

Ack by me then!

As well, maybe it would be useful to do the same for display_attr() as well.

commit a9a942a35f82e00fa46ade19e78e4e782dab136f
To ssh://git@pagure.io/lib389.git
60fb853..a9a942a master -> master

get_attr_val_int("attr") raises an error if attr doesn't exist:

Error: int() argument must be a string, a bytes-like object or a number, not 'NoneType'

The other functions will return None in that case.

How should we take care of it?

Hmmm interesting. I think the issue is that we return a "None" at all. Maybe something in _entry is doing this?

Perhaps we have to look for this case in all the checks, because we can't byte / utf encode none either.

In the other cases we use ensure_str/ensure_bytes which check against None: https://pagure.io/lib389/blob/master/f/lib389/utils.py#_786

We can do the check against None either in a get_attr_val_int() or we can create a new ensure_int() function.

However, I think first we have to decide what we want to do. Do we want to return None from all of these functions if the requested attribute doesn't exist?

Hmmm. Well, the current behaviour is that the caller of get_attr_val expects a "thing" or a None to say it's not there. So I think perhaps we should keep this interface, as we have built the expectation of this behaviour.

In that case, we should be checking for None, and skipping over it - as you say, with ensure_int or something similar.

Took the chance as well to replace some "from foo import *" imports with explicit ones:

Oops, I broke something with the last one. Fixed:

commit b28ef1862b0a69942cd664733e4e8c3ebf3715ad
To ssh://git@pagure.io/lib389.git
b5a8279..b28ef18 master -> master

Great work, thank you!