use reference keys instead of relations #4410
Conversation
|
integration tests won't really hit all the changes here. I'm going to make sure they pass locally with |
|
It looks like the ListRelations event takes a list of relations, so we need to run the make_key thing on each element of the list |
| @@ -2499,8 +2491,8 @@ def message(self) -> str: | |||
| SQLQueryStatus(status="", elapsed=0.1) | |||
| SQLCommit(conn_name="") | |||
| ColTypeChange(orig_type="", new_type="", table="") | |||
| SchemaCreation(relation=BaseRelation()) | |||
| SchemaDrop(relation=BaseRelation()) | |||
| SchemaCreation(relation=_make_key(BaseRelation())) | |||
There was a problem hiding this comment.
I would have expected these lines to be: SchemaCreation(relation=_ReferenceKey). Is there some reason to call the _make_key function here?
There was a problem hiding this comment.
it's just shorter. this code is never run, just evaluated by mypy. happy to change it though if you think it's better.
|
it's failing integration tests with json formatted logs locally so I'll be pushing a few more changes to fix those issues. |
|
now it hangs here when you can trigger this locally with |
|
I'm going to merge anyway to get these fixes in. I can open a new pr to fix this hang once I can nail it down. |
Co-authored-by: Nathaniel May <[email protected]>
automatic commit by git-black, original commits: 9bdf5fe
Description
our Relation types override the behavior of deepcopy which means they cannot go through the json serialization pipeline which leverages asdict. This is because the dict_factory that asdict takes is not applied till the end of processing when deepcopy is internally used. The way we've overridden the behavior raises an exception when we try to serialize these values as part of a dataclass. Using reference keys instead is easier to serialize since it's just a named tuple.
Checklist
CHANGELOG.mdand added information about my change