Skip to content

fix: give better error message when object not serializable#3861

Merged
tobymao merged 1 commit intomainfrom
toby/serialize_class
Feb 19, 2025
Merged

fix: give better error message when object not serializable#3861
tobymao merged 1 commit intomainfrom
toby/serialize_class

Conversation

@tobymao
Copy link
Contributor

@tobymao tobymao commented Feb 18, 2025

No description provided.

@tobymao tobymao requested a review from georgesittas February 18, 2025 23:32
@tobymao tobymao force-pushed the toby/serialize_class branch from 436bb1d to 75c85e4 Compare February 18, 2025 23:38
@tobymao tobymao force-pushed the toby/serialize_class branch 2 times, most recently from d5f00a8 to 74ea662 Compare February 19, 2025 03:07
Copy link
Contributor

@georgesittas georgesittas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, let's fix the title before merging because it's no longer true.

Comment on lines +463 to +465
raise SQLMeshError(
f"'{v}' cannot be serialized because it is a constant object. Import the module and call it from within the macro instead:\nimport module\n\ndef my_macro():\n module.{v}"
)
Copy link
Contributor

@georgesittas georgesittas Feb 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This applies for models, signals too, besides macros, so the message could be misleading. I'd do this instead:

Suggested change
raise SQLMeshError(
f"'{v}' cannot be serialized because it is a constant object. Import the module and call it from within the macro instead:\nimport module\n\ndef my_macro():\n module.{v}"
)
raise SQLMeshError(
f"Object '{v}' cannot be serialized. If it's defined in a library, import the corresponding "
"module and reference the object using its fully-qualified name. For example, the datetime "
"module's 'UTC' object should be accessed as 'datetime.UTC'."
)

This would print something like:

Object 'UTC' cannot be serialized. If it's defined in a library, import the corresponding module and reference the object using its fully-qualified name. For example, the datetime module's 'UTC' object should be accessed as 'datetime.UTC'.


IGNORE_DECORATORS = {"macro", "model", "signal"}
SERIALIZABLE_CALLABLES = (type, types.FunctionType)
LITERALS = (Number, str, bytes, tuple, list, dict, set, bool)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can also add Ellipsis here for completeness. According to literal_eval's docs:

Evaluate an expression node or a string containing only a Python literal or container display. The string or node provided may only consist of the following Python literal structures: strings, bytes, numbers, tuples, lists, dicts, sets, booleans, None and Ellipsis.

@tobymao tobymao changed the title fix: make sure to serialize classes like UTC as well fix: give better error message when object not serializable Feb 19, 2025
@tobymao tobymao force-pushed the toby/serialize_class branch from 74ea662 to dca4b20 Compare February 19, 2025 16:34
@tobymao tobymao merged commit eae7027 into main Feb 19, 2025
21 checks passed
@tobymao tobymao deleted the toby/serialize_class branch February 19, 2025 19:39
justinjoseph89 added a commit to trygforsikring/sqlmesh that referenced this pull request Feb 20, 2025
* Chore: replace mysql-connector-python with pymysql (TobikoData#3788)

* feat: don't force db connect if using serverless (TobikoData#3786)

* Feat: Add 'auditing' and 'promoting' runtime stages (TobikoData#3791)

* fix: databricks set default catalog for both connections (TobikoData#3793)

* Docs: Prod Env Observability updates (TobikoData#3790)

Co-authored-by: Trey Spiller <1831878+treysp@users.noreply.github.com>
Co-authored-by: Trey Spiller <treyspiller@gmail.com>

* Docs: update jinja gateway variable syntax (TobikoData#3795)

* Fix: Only promote all snapshots if the target environment expired (TobikoData#3797)

* Chore: remove [missing dates] from CLI model backfills title (TobikoData#3796)

* Fix: Make sure that physical tables exist for promoted snapshots (TobikoData#3798)

* fix: db properly support `with_log_level` (TobikoData#3799)

* fix: run integration tests in session (TobikoData#3801)

* Fix: specify init duckdb database so quickstart works (TobikoData#3800)

* Docs: clarify in faq that run ignores local definitions  (TobikoData#3794)

* feat: add airflow operator and hook for ClickHouse (TobikoData#3699)

* Feat: Allow macros in python model properties (TobikoData#3740)

* Docs: update CLI quickstart's CLI output (TobikoData#3802)

* Fix: Ensure diff sample displays when table names have been uppercased (TobikoData#3806)

* Fix: update github links for pdoc api docs (TobikoData#3807)

* Feat: Run audits as non-blocking on dev previews (TobikoData#3809)

* Chore!: bump sqlglot to v26.6.0 (TobikoData#3810)

* Feat!: Adjust physical_properties evaluation and add macro to resolve physical table names (TobikoData#3772)

* Chore: Use 'dev' suffix instead of 'temp' for non-deployable physical tables (TobikoData#3803)

* Fix!: Propagate the grain attribute when converting dbt models (TobikoData#3804)

* Docs(dagster): Fix installation instructions (TobikoData#3812)

* fix: add kwargs to build_table_properties_exp (TobikoData#3817)

* Feat: include alter statements in destructive change error message (TobikoData#3805)

* Dagster demo and tutorial video (TobikoData#3822)

* fix: databricks with_log_level (TobikoData#3823)

* Feat: improve audit error message formatting (TobikoData#3818)

* Fix(postgres): Quote role names if required when running SET ROLE on cursor init (TobikoData#3825)

* Fix: handle quoted projects properly in bigquery adapter (TobikoData#3820)

* Chore: Fix flaky test (TobikoData#3828)

* docs: adding self hosted executor docs (TobikoData#3816)

* Revert "Fix(postgres): Quote role names if required when running SET ROLE on cursor init" (TobikoData#3834)

* Fix: Don't fail because of an unrestorable change if the target model is forward-only (TobikoData#3835)

* Chore: improve metadata update console printing (TobikoData#3824)

* fix: respect disable_restatement remove intervals across env (TobikoData#3838)

* fix: respect disable restate dev unpaused snapshots (TobikoData#3840)

* Feat: add [WARNING] to console warning messages (TobikoData#3826)

* Feat: Extend support of project wide model properties (TobikoData#3832)

* Fix: Streamline execution of pre- / post- statements when creating a physical table (TobikoData#3837)

* fix: signals that return an empty list are considered ready (TobikoData#3841)

* Chore: Add the ingress section to the self-hosted executor docs

* Fix: Snapshots promoted in prod shouldn't be restated in dev (TobikoData#3843)

* Fix: Inference of python model names from the file system (TobikoData#3844)

* feat: add support for datetime/date in macros (TobikoData#3846)

* fix: only expand restatement range if incremental (TobikoData#3847)

* feat: improve gcp postgres connection config options (TobikoData#3842)

* Fix: Pin PyGithub to 2.5.0 so tests can run (TobikoData#3851)

* Fix: Warn when SQLMesh automatically adjusts a restatement range to cover the whole model (TobikoData#3850)

* fix!: normalize catalog override name (TobikoData#3849)

* Feat: allow different warning messages for logger and console (TobikoData#3836)

* Chore: fix audit doc typos (TobikoData#3856)

* Add airflow tutorial video (TobikoData#3860)

* Feat!: add model blueprinting (TobikoData#3848)

* Fix: Make sure that pending restatement intervals are always recorded last during compaction (TobikoData#3862)

* Chore: Break up the plan_builder method in Context (TobikoData#3867)

* fix: give better error message when object not serializable (TobikoData#3861)

* Feat: make date_spine macro less strict to allow dynamic behavior (TobikoData#3865)

* Add Tcloud SSO docs (TobikoData#3827)

* Fix: Unexpected backfill of a parent of a changed forward-only child when the child runs before the parent (TobikoData#3871)

* Feat: Allow CustomKind subclasses for custom materializations (TobikoData#3863)

* Chore: consolidate `make install-*` (TobikoData#3874)

---------

Co-authored-by: Trey Spiller <1831878+treysp@users.noreply.github.com>
Co-authored-by: Ryan Eakman <6326532+eakmanrq@users.noreply.github.com>
Co-authored-by: Iaroslav Zeigerman <zeigerman.ia@gmail.com>
Co-authored-by: Marisa Smith <66020208+mesmith027@users.noreply.github.com>
Co-authored-by: Trey Spiller <treyspiller@gmail.com>
Co-authored-by: Anton Parfenyuk <spar9a@gmail.com>
Co-authored-by: Themis Valtinos <73662635+themisvaltinos@users.noreply.github.com>
Co-authored-by: Gerasimos Kounadis <gerasimoskounadis@gmail.com>
Co-authored-by: Erin Drummond <erin.dru@gmail.com>
Co-authored-by: Jo <46752250+georgesittas@users.noreply.github.com>
Co-authored-by: Sung Won Chung <sungwonchung3@gmail.com>
Co-authored-by: Ben <9087625+benfdking@users.noreply.github.com>
Co-authored-by: Philippe Laflamme <484152+plaflamme@users.noreply.github.com>
Co-authored-by: Toby Mao <toby.mao@gmail.com>
Co-authored-by: Afzal Jasani <amj355@nyu.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants