feat: upgrade to SQLMesh 0.209.0 #49
Conversation
|
@richban thank you so much for the PR! I'll review this hopefully in the next day or so. I actually would like to relieve the version pinning now. In the meantime, we had to fix a bug is it possible to rebase? |
- Add proper *args handling in IntrospectingConsole.create_signatures_and_params() - Convert *args to _unknown_args_from_varargs dict to bridge incompatible interfaces - Update publish_known_event() to process converted *args data - Fix LogTestResults field ordering to prevent syntax errors - Update SQLMesh constraint from <0.188 to >=0.188 Fixes compatibility issue with SQLMesh 0.209.0 which added *args/**kwargs to log_error() and log_warning() abstract methods. Without this fix, the dynamic method generation creates invalid Python syntax that prevents import. The solution preserves all argument data in unknown_args while maintaining the clean **kwargs-only interface of the event system. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
|
@richban I've been thinking about this a bunch mostly cause I didn't like my current solution that relies on Something like: class GeneratedCallable(t.Generic[EventType]):
def __init__(self, console: IntrospectingConsole, event_cls: type[EventType], original_signature: inspect.Signature):
self.console = console
self.event_cls = event_cls
self.original_signature = original_signature
def __call__(self, *args: t.Any, **kwargs: t.Any) -> None:
"""Create an instance of the event class with the provided arguments."""
... generic call logic here that validates the type... I think this would satisfy a few things:
Is this something you think you'd be able to change here? If not, I could also fix this as well, I just haven't had a moment recently. |
|
Related: dagster-io/dagster#32092 |
|
Ah shoot, the changes don’t look bad but commits aren’t passing the gitlint ci check. This needs to use conventional commits to pass. |
|
Got it! |
|
@richban i just noticed, |
Replace dynamic exec() generation with proper callable class pattern for better maintainability, type safety, and debugging capability. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
| func_signature, call_params = cls.create_signatures_and_params(signature) | ||
| def create_unknown_event_handler(cls, method_name: str, signature: inspect.Signature) -> t.Callable[..., None]: | ||
| """Create an UnknownEventCallable for unknown events.""" | ||
| def handler(self: IntrospectingConsole, *args: t.Any, **kwargs: t.Any) -> None: |
There was a problem hiding this comment.
We can clean this up a bit and set the generated callable as the response. No need to wrap this in the another callable.
| func_signature, call_params = cls.create_signatures_and_params(signature) | ||
| def create_event_handler(cls, method_name: str, event_cls: type[BaseConsoleEvent], signature: inspect.Signature) -> t.Callable[..., None]: | ||
| """Create a GeneratedCallable for known events.""" | ||
| def handler(self: IntrospectingConsole, *args: t.Any, **kwargs: t.Any) -> None: |
There was a problem hiding this comment.
Same issue no need to wrap the callable in a callable
There was a problem hiding this comment.
Let me know if the new changes make sense.
dagster_sqlmesh/console.py
Outdated
| # Create methods now that we have self | ||
| for method_type, method_name, event_cls, signature in self._method_info: | ||
| if method_type == 'known': | ||
| handler = GeneratedCallable(self, event_cls, signature, method_name) |
There was a problem hiding this comment.
I am a bit old school this way, but I don't like to do things in constructors that might fail if I can really help it. Can we instead put the logic to handle the methods back. I think perhaps telling you not to wrap in that handler made it not possible to use __init_subclass__? If so, let's just keep that wrapped handler then.
There was a problem hiding this comment.
when I tried to eliminate the wrapper handler, it made __init_subclass__ impossible to use because we needed self
|
@richban some additional issues on the git commit lint. Apologies, I'm just gonna close this and fix that up myself so I can get this across the finish line! |
|
Thanks for the feedback and patience. |
… console and backfilling integration\n- Support latest Dagster and SQLMesh versions\n- Exclude private README changes; keep upstream README\n\nRefs: opensource-observer#49, closes opensource-observer#43; follow-up to opensource-observer#52
2 participants
Refactor: Replace exec() usage with GeneratedCallable classes
Problem
The current dynamic console method generation uses
exec()to create string-based Python code, causing invalid syntax with args/kwargs parameters and breaking SQLMesh0.209.0compatibility.Solution
Replace
exec()approach withGeneratedCallableandUnknownEventCallableclasses that use Python's callable protocol to handle method invocations directly.Changes
GeneratedCallableandUnknownEventCallableclassesIntrospectingConsole.create_event_handler()to use callablescreate_signatures_and_params()string generation methodtextwrapimportBenefits
Dependencies
>=0.188→0.209.03.11-3.12support maintained