> I'd view calling it directly as a layering violation.  A Datum doesn't
> currently know anything about the IDL.  Making it know directly about
> ovs.db.idl.Row and what to do about it seems like a bad idea.
>
> If you'd prefer, I'll move to_python() into the Idl code as an
> internal helper, in which case no uuid_to_row argument is needed.
> That's the way I had it originally, but then I thought that it was
> more naturally a method on Datum.

I don't have a preference in particular I suppose.  Most users won't
be directly calling to_python() directly will they?  If it's mostly
used internally in the idl I think it doesn't matter.



>        """Returns a new Datum with the given ovs.db.types.Type 'type_'.  The
>        new datum's value is taken from 'value', which must take the form
>        described as a valid return value from Datum.to_python() for 'type'.
>
>        Each scalar value within 'value' is initally passed through
>        'row_to_uuid', which should convert objects that represent rows (if
>        any) into uuid.UUID objects and return other data unchanged.
>
>        Raises ovs.db.error.Error if 'value' is not in an appropriate form for
>        'type_'."""

Looks good.

> The caller is allowed to add attributes to schema columns (in
> particular, 'alert') that ovs.db.schema.DbSchema won't copy.

Ah, I didn't realize that couldn't happen after the constructor was
called.  Seems fine then.

> OK, would you mind doing that as part of adding the new user?  This
> series is long enough already.

Yep no problem.

>    string_type = types.Type.from_json("string")
>    string_map_type = types.Type.from_json(["key": "string",
>                                            "value": "string",
>                                            "min": 0,
>                                            "max": "unlimited"])
>
> Or we could just not bother with validating the types anymore, since
> they're part of the distribution instead of retrieved remotely from
> the database now.

I think using from_json as described above is a readable way to do it.
 I don't really have an opinion on whether or not we should be
validating the types in the first place.  If you think there is a
value in it then we should probably leave it.

Ethan
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev

Reply via email to