[mlbiche]

As for Select query, it adds exclude_secrets to Objects query.

[mlbiche]

Should we compare to None or to something else? Such as ...?

[mlbiche]

The test is not passing because password has a default value being an empty string (that is then hashed). The default value is affected at Table class initialization:

piccolo/piccolo/table.py

Lines 492 to 505 in fc1fc86

	if value is ...:
	value = column.get_default_value()
	if isinstance(value, Default):
	value = value.python()
	if (
	(value is None)
	and (not column._meta.null)
	and not _ignore_missing
	):
	raise ValueError(f"{column._meta.name} wasn't provided")
	self[column._meta.name] = value

... being called from Query:

piccolo/piccolo/query/base.py

Lines 122 to 128 in fc1fc86

	return cast(
	QueryResponseType,
	[
	self.table(**columns, _exists_in_db=True)
	for columns in raw
	],
	)

An option would be to set _ignore_missing argument to True and to only consume the default value if _ignore_missing is True. But I am not sure about other impacts this could have?

[mlbiche]

Ok, I have just tried this implementation.

[mlbiche]

If ... as a default value is not ok, what about turning this into:

if value is not ...:
    self[column._meta.name] = value

However, it means that the column may not be available in self so the __get__ method in piccolo/columns/column_types.py has to be adapted accordingly.

[dantownsend]

I think the challenge with exclude_secrets and objects queries is let's say you do this:

class User(Table):
    name = Varchar()
    password = Varchar(secret=True)

user = await User.objects(exclude_secrets=True).first()

By excluding secrets, would password be set to None on the user object?

If we then save the object, it has the potential to override the stored password.

await user.save()

So we'd have to assign something like SecretValue() or ... or something to password, and then when saving it, these values get stripped out.

[mlbiche]

By excluding secrets, would password be set to None on the user object?

That is exactly what I am wondering in my previous comments. 🙂 For now, I'm leaning towards ... but maybe a SecretValue would be better. None has to be exclude for me as it's then impossible to make the difference between a null value or a not set value.

If we then save the object, it has the potential to override the stored password.

That's a good catch! It is exactly what is breaking tests for now. I am going to try to complete test_get_exclude_secrets test case with this check and try to make it work.

[dantownsend]

By excluding secrets, would password be set to None on the user object?

That is exactly what I am wondering in my previous comments. 🙂 For now, I'm leaning towards ... but maybe a SecretValue would be better. None has to be exclude for me as it's then impossible to make the difference between a null value or a not set value.

If we then save the object, it has the potential to override the stored password.

That's a good catch! It is exactly what is breaking tests for now. I am going to try to complete test_get_exclude_secrets test case with this check and try to make it work.

Yes, something like SecretValue or ... sounds good. Maybe a custom class like SecretValue, to make it even more explicit, but whatever works.

[mlbiche]

Here is a proposition for the save method. However, I am not entirely satisfied of reusing _ignore_missing. What about introducing a parameter _ignore_secrets that would be used in the Table initialization?

[mlbiche]

Indeed, it is breaking the ModelBuilder...