python(feat): udf resource - #793
Brandon-Shippy wants to merge 1 commit into
Conversation
|
Python docs preview: https://sift-stack.github.io/sift/python/pr-793/ Deployed from |
| updated = await self._low_level_client.update_function(update) | ||
| return self._apply_client_to_instance(updated) | ||
|
|
||
| async def sync( |
There was a problem hiding this comment.
I'd drop sync, or if you want to keep it, rename it to something like create_or_update and look up by names instead of listing the whole org, skip archived matches, carry user_notes through, and document that the server blocks input, output type, and name changes on in-use functions, so a rerun can't promise Sift will match the file
I think we should drop it, since anyone who needs it can write find then create or update in a few lines and see exactly where a partial failure stopped
There was a problem hiding this comment.
I agree we should remove this. It is not idiomatic with the rest of the types.
| """Base class for UserDefinedFunction create and update models.""" | ||
|
|
||
| description: str | None = None | ||
| function_inputs: list[FunctionInput] | None = None |
There was a problem hiding this comment.
The server rejects an empty function_inputs on create and validate, so this should be required on UserDefinedFunctionCreate and validate()
| ) | ||
|
|
||
| async def dependents( | ||
| self, function: str | UserDefinedFunction, *, version_id: str | None = None |
There was a problem hiding this comment.
if version_id is passed, function gets ignored. Is this intentional?
| ).GetUserDefinedFunctionDependents(request) | ||
| response = cast("GetUserDefinedFunctionDependentsResponse", response) | ||
|
|
||
| return FunctionDependents( |
There was a problem hiding this comment.
Was there a reason to reduce these to IDs? The response returns full messages and _from_proto exists for all three types
| function: str | UserDefinedFunction, | ||
| update: UserDefinedFunctionUpdate | dict, | ||
| ) -> UserDefinedFunction: | ||
| """Update a function. This creates a new version. |
There was a problem hiding this comment.
Two things here. Archives return the current version, so something like Changes to the expression, inputs, description, or metadata create a new version. would be more accurate. And let's add a user_notes kwarg like calculated channels have, threaded into the wrapper and set on the proto after to_proto_with_mask(). The server reads it from the body, and right now there's no way to leave a note on a version
There was a problem hiding this comment.
One catch for user_notes, the server treats an empty note as a change, so when none is given, send the current version's note rather than empty, otherwise every update clears it and bumps the version. Rules update already fetches the object when passed an ID, so the same pattern works here
| async def list_( | ||
| self, | ||
| *, | ||
| function: str | UserDefinedFunction | None = None, |
There was a problem hiding this comment.
missing a lot of other filters we should support here.
| name_contains: str | None = None, | ||
| name_regex: str | re.Pattern | None = None, | ||
| # self ids | ||
| function_ids: list[str] | None = None, |
There was a problem hiding this comment.
object and Id support.
A lot of missing filters here?
| results.append(await self.update(current, changes)) | ||
| return results | ||
|
|
||
| async def validate( |
There was a problem hiding this comment.
validate_expression - more descriptive naming
| expression=expression, function_inputs=function_inputs | ||
| ) | ||
|
|
||
| async def dependents( |
There was a problem hiding this comment.
get_where_used is more clear IMO
| ) | ||
|
|
||
| async def dependents( | ||
| self, function: str | UserDefinedFunction, *, version_id: str | None = None |
There was a problem hiding this comment.
version: UserDefinedFunctionVersion | str
Also since version is fully defined, it should make passing the function itself optional
| ) | ||
|
|
||
| async def dependents( | ||
| self, function: str | UserDefinedFunction, *, version_id: str | None = None |
There was a problem hiding this comment.
nit on naming: user_defined_function instead of function
|
|
||
| identifier: str | ||
| data_type: FunctionDataType = FunctionDataType.NUMERIC | ||
| constant: bool = False |
There was a problem hiding this comment.
This naming is confusing. It isn't a constant since that would imply it doesn't change, rather this is a value provided by the caller. scalar or something may make more sense.
| ) | ||
|
|
||
| @property | ||
| def versions(self) -> list[UserDefinedFunction]: |
There was a problem hiding this comment.
Please add a latest_version
| name: str | ||
| description: str | ||
| expression: str | ||
| version: int |
There was a problem hiding this comment.
This is pretty confusing and repeats some of the same sins of say Channels.
Instead of a single object, we should have two:
- User Defined Function
- User Defined Function Version
The User Defined Function should contain a reference to the latest version, but be version agnostic
| """Create model for UserDefinedFunction.""" | ||
|
|
||
| name: str | ||
| expression: str |
There was a problem hiding this comment.
Should be documented that this is a cel expression
|
|
||
| name: str | ||
| expression: str | ||
| user_notes: str | None = None |
There was a problem hiding this comment.
Rename this "change_notes" or something more clear what it is doing
| output_type: The type the expression returns, when it is valid. | ||
| """ | ||
|
|
||
| valid: bool |
|
|
||
|
|
||
| class FunctionDependents(BaseModel): | ||
| """What would break if a function changed. |
There was a problem hiding this comment.
Is that what this represents? Why would things necessarily break?
| """ | ||
|
|
||
| function_ids: list[str] = [] | ||
| calculated_channel_ids: list[str] = [] |
There was a problem hiding this comment.
add properties to return the objects
This branch has not been deployed
3 participants
Description
Adds a user-defined functions resource to
sift_client. A function is a reusable, versioned expression that calculated channels and rules call by name.UserDefinedFunctionsAPIAsyncwithget,list_,find,create,update,archive,unarchive. Wired up asclient.user_defined_functionsUserDefinedFunctionVersionsAPIAsyncnested asclient.user_defined_functions.versionsUserDefinedFunctionsLowLevelClientwrapsUserDefinedFunctionService; the high-level client has zero proto importsPydantic models in
sift_types/user_defined_function.pyvalidate(...)checks an expression without saving itdependents(...)returns the rules and calculated channels that would break on a changesync(...)creates or updates a list of definitions by name, so a definitions file is rerunnableSync wrappers and type stubs registered
Validation