feat: Add support for serialization and tracing metadata params#452
feat: Add support for serialization and tracing metadata params#452jacoblee93 wants to merge 2 commits into
Conversation
|
Just documenting one behavioral effect of this PR for anyone reading later — no action needed: This change causes the Databricks workspace host to be emitted into trace/run metadata on every invocation. The path:
Two clarifications:
The host is workspace-identifying but not a secret (the token is correctly kept out of this path), so this is just a heads-up on the new trace content rather than a concern. |
aravind-segu
left a comment
There was a problem hiding this comment.
Added a quick question. Did we do an E2E test here to see if everything works here?
| """Name of Databricks Model Serving endpoint to query.""" | ||
| target_uri: Optional[str] = None | ||
| """The target MLflow deployment URI to use. Deprecated: use workspace_client instead.""" | ||
| databricks_host: Optional[str] = None |
There was a problem hiding this comment.
We ask users to pass in the workspace_client, is there a specific reason to explicitly pass in databricks_host and token ?
There was a problem hiding this comment.
Our service must recreate chat model instances from serialized chat model runs. We cannot easily represent complex objects like client instances, so it's convenient here to add an initialization path that uses only simple primitives like strings.
|
Yes, have tested end to end |
aravind-segu
left a comment
There was a problem hiding this comment.
Thanks for the change, lgtm!
|
Thanks for the clarifications offline! |
Adds support for serialization of runs and populates useful metadata parameters for tracing