-
Notifications
You must be signed in to change notification settings - Fork 253
Add on_change callbacks for managed variables #1990
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 3 commits
6c310d0
d36e24e
a87366a
6a7a49d
19c9578
b0705e5
57b1543
dcb5fa6
2c107a2
378c021
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2603,6 +2603,7 @@ def var( | |
| description=description, | ||
| ) | ||
| self._variables[name] = variable | ||
| self._config.add_variables_change_listener(self._on_variables_config_change) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think #2034 should be merged first, and then this should change so that Logfire doesn't need to own callbacks. then there'll be 3 layers of callbacks instead of 4. |
||
|
|
||
| from logfire.variables.variable import warn_on_template_inputs_composition_mismatch | ||
|
|
||
|
|
@@ -2740,13 +2741,36 @@ class PromptInputs(BaseModel): | |
| template_mismatch_policy=template_mismatch_policy, | ||
| ) | ||
| self._variables[name] = variable | ||
| self._config.add_variables_change_listener(self._on_variables_config_change) | ||
|
|
||
| from logfire.variables.variable import warn_on_template_inputs_composition_mismatch | ||
|
|
||
| warn_on_template_inputs_composition_mismatch(self._variables, variable) | ||
|
|
||
| return variable | ||
|
|
||
| def _on_variables_config_change(self, changed_names: set[str]) -> None: | ||
| """Dispatch variable config changes to registered variables' on_change callbacks. | ||
|
|
||
| Registered with this instance's `LogfireConfig` (which wires it to every provider it | ||
| creates) the first time a variable is defined. Expands the directly-changed names to | ||
| every registered variable that is *effectively* changed — including variables that | ||
| (transitively) compose a changed variable via `@{ref}@` references — then fires each | ||
| affected variable's callbacks. | ||
| """ | ||
| # Snapshot the registry: dispatch runs on the provider's polling thread, and a | ||
| # concurrent `var()` on another thread mutating the dict mid-iteration would | ||
| # otherwise raise (losing the rest of this change cycle's notifications). | ||
| variables = dict(self._variables) | ||
| if not variables: | ||
| return | ||
|
|
||
| from logfire.variables.variable import expand_config_changes | ||
|
|
||
| provider_config = self.config.get_variable_provider().get_all_variables_config() | ||
| for name in sorted(expand_config_changes(changed_names, provider_config, variables)): | ||
| variables[name]._notify_change() # pyright: ignore[reportPrivateUsage] | ||
|
|
||
| def variables_clear(self) -> None: | ||
| """Clear all registered variables from this Logfire instance. | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,8 @@ | |
| VariableAlreadyExistsError, | ||
| VariableNotFoundError, | ||
| VariableProvider, | ||
| changed_config_keys, | ||
| resolution_relevant_config_changed, | ||
| ) | ||
| from logfire.variables.config import VariableConfig, VariablesConfig | ||
|
|
||
|
|
@@ -96,6 +98,7 @@ def create_variable(self, config: VariableConfig) -> VariableConfig: | |
| raise VariableAlreadyExistsError(f"Variable '{config.name}' already exists") | ||
| self._config.variables[config.name] = config | ||
| self._config._invalidate_alias_map() # pyright: ignore[reportPrivateUsage] | ||
| self._notify_config_change(changed_config_keys(config)) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Snapshot configs before diffing notifications.
Suggested boundary-copy shape def get_variable_config(self, name: str) -> VariableConfig | None:
...
with self._lock:
- return self._config._get_variable_config(name) # pyright: ignore[reportPrivateUsage]
+ config = self._config._get_variable_config(name) # pyright: ignore[reportPrivateUsage]
+ return config.model_copy(deep=True) if config is not None else None
def create_variable(self, config: VariableConfig) -> VariableConfig:
...
with self._lock:
+ config = config.model_copy(deep=True)
if config.name in self._config.variables:
raise VariableAlreadyExistsError(f"Variable '{config.name}' already exists")
self._config.variables[config.name] = config
self._config._invalidate_alias_map() # pyright: ignore[reportPrivateUsage]
def update_variable(self, name: str, config: VariableConfig) -> VariableConfig:
...
with self._lock:
if name not in self._config.variables:
raise VariableNotFoundError(f"Variable '{name}' not found")
- old_config = self._config.variables[name]
- self._config.variables[name] = config
+ old_config = self._config.variables[name].model_copy(deep=True)
+ config = config.model_copy(deep=True)
+ self._config.variables[name] = config
self._config._invalidate_alias_map() # pyright: ignore[reportPrivateUsage]
def delete_variable(self, name: str) -> None:
...
- old_config = self._config.variables.pop(name)
+ old_config = self._config.variables.pop(name).model_copy(deep=True)Also applies to: 120-124, 139-141 🤖 Prompt for AI Agents |
||
| return config | ||
|
|
||
| def update_variable(self, name: str, config: VariableConfig) -> VariableConfig: | ||
|
|
@@ -114,8 +117,11 @@ def update_variable(self, name: str, config: VariableConfig) -> VariableConfig: | |
| with self._lock: | ||
| if name not in self._config.variables: | ||
| raise VariableNotFoundError(f"Variable '{name}' not found") | ||
| old_config = self._config.variables[name] | ||
| self._config.variables[name] = config | ||
| self._config._invalidate_alias_map() # pyright: ignore[reportPrivateUsage] | ||
| if resolution_relevant_config_changed(old_config, config): | ||
| self._notify_config_change(changed_config_keys(old_config, config)) | ||
| return config | ||
|
|
||
| def delete_variable(self, name: str) -> None: | ||
|
|
@@ -130,5 +136,6 @@ def delete_variable(self, name: str) -> None: | |
| with self._lock: | ||
| if name not in self._config.variables: | ||
| raise VariableNotFoundError(f"Variable '{name}' not found") | ||
| del self._config.variables[name] | ||
| old_config = self._config.variables.pop(name) | ||
| self._config._invalidate_alias_map() # pyright: ignore[reportPrivateUsage] | ||
| self._notify_config_change(changed_config_keys(old_config)) | ||
Uh oh!
There was an error while loading. Please reload this page.