feat: Deprecate host_definition
#29222
Conversation
This was an attempt to collect all domains so that we could use it on Web Analytics' UI. We've decided to go a different route because of the problems we'd have with having this inside `propdefs`. We don't usually drop any tables from our DB, so let's simply tag it as deprecated. We've also implemented a new `@deprecated` decorator for python for this usecase, we're using it for a different already-deprecated class as well
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR introduces a new @deprecated decorator in posthog/utils.py to standardize how deprecated components are marked in the codebase.
- Added a flexible
@deprecateddecorator that works with both classes and functions, supporting optional reason messages - Applied the decorator to
HostDefinitionmodel with a clear explanation about performance issues with propdefs - Converted the existing
Alertclass deprecation from a docstring comment to use the new decorator - The decorator emits warnings when deprecated code is used, improving visibility of deprecated components
- Changed direct model imports to string references in
HostDefinitionto avoid circular import issues
3 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile
There was a problem hiding this comment.
I am not sure what to do with the python code quality issues, they seem reasonable enough to fix but for the nature of the decorator, could be ok to ignore as well.
What were you planning to do @rafaeelaudibert ?
| @@ -1536,3 +1538,47 @@ def get_from_dict_or_attr(obj: Any, key: str): | |||
| return getattr(obj, key, None) | |||
| else: | |||
| raise AttributeError(f"Object {obj} has no key {key}") | |||
|
|
|||
|
|
|||
| def deprecated(reason: str | None = None): | |||
There was a problem hiding this comment.
Python 3.13 actually added this as a built in decorator, there is a link for the docs here and the corresponding PEP: https://docs.python.org/3.13/library/warnings.html#warnings.deprecated
Maybe we could keep the method signature to make this future-proof?
This was an attempt to collect all domains so that we could use it on Web Analytics' UI. We've decided to go a different route because of the problems we'd have with having this inside
propdefs. We don't usually drop any tables from our DB, so let's simply tag it as deprecated.We've also implemented a new
@deprecateddecorator for python for this usecase, we're using it for a different already-deprecated class as well