Skip to content
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

make sure LazyTableReference isn't resolved until referenced table has been initialised #758

Draft
wants to merge 21 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion docs/src/piccolo/api_reference/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ LazyTableReference
.. currentmodule:: piccolo.columns

.. autoclass:: LazyTableReference
:members:

-------------------------------------------------------------------------------

Expand Down
76 changes: 58 additions & 18 deletions piccolo/apps/migrations/auto/serialisation.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
from piccolo.columns import Column
from piccolo.columns.defaults.base import Default
from piccolo.columns.reference import LazyTableReference
from piccolo.table import Table
from piccolo.table import Table, create_table_class
from piccolo.utils.repr import repr_class_instance

from .serialisation_legacy import deserialise_legacy_params
Expand Down Expand Up @@ -483,6 +483,31 @@ def __repr__(self):
###############################################################################


def _primary_key_params(primary_key: Column) -> SerialisedParams:
"""
Returns the extra imports and definitions required for a primary column.
"""
primary_key_class = primary_key.__class__

pk_serialised_params: SerialisedParams = serialise_params(
params=primary_key._meta.params
)

pk_serialised_params.extra_imports.append(
Import(
module=primary_key_class.__module__,
target=primary_key_class.__name__,
expect_conflict_with_global_name=getattr(
UniqueGlobalNames,
f"COLUMN_{primary_key_class.__name__.upper()}",
None,
),
)
)

return pk_serialised_params


def serialise_params(params: t.Dict[str, t.Any]) -> SerialisedParams:
"""
When writing column params to a migration file, we need to serialise some
Expand Down Expand Up @@ -664,12 +689,24 @@ def serialise_params(params: t.Dict[str, t.Any]) -> SerialisedParams:
expect_conflict_with_global_name=UniqueGlobalNames.TABLE,
)
)
pk_serialised_params = _primary_key_params(
table_type._meta.primary_key
)
extra_imports.extend(pk_serialised_params.extra_imports)
extra_definitions.extend(pk_serialised_params.extra_definitions)
continue

# Replace any Table class values into class and table names
if inspect.isclass(value) and issubclass(value, Table):
params[key] = SerialisedCallable(callable_=value)
extra_definitions.append(SerialisedTableType(table_type=value))
if key == "references" and isinstance(value, str):
# This isn't perfect - it's better if the user uses
# LazyTableReference, but do the best we can.

referenced_table_class = create_table_class(
class_name=value.rsplit(".")[-1]
)

extra_definitions.append(
SerialisedTableType(table_type=referenced_table_class)
)
extra_imports.append(
Import(
module=Table.__module__,
Expand All @@ -678,23 +715,26 @@ def serialise_params(params: t.Dict[str, t.Any]) -> SerialisedParams:
)
)

primary_key_class = value._meta.primary_key.__class__
pk_serialised_params = _primary_key_params(
referenced_table_class._meta.primary_key
)
extra_imports.extend(pk_serialised_params.extra_imports)
extra_definitions.extend(pk_serialised_params.extra_definitions)
continue

# Replace any Table class values into class and table names
if inspect.isclass(value) and issubclass(value, Table):
params[key] = SerialisedCallable(callable_=value)
extra_definitions.append(SerialisedTableType(table_type=value))
extra_imports.append(
Import(
module=primary_key_class.__module__,
target=primary_key_class.__name__,
expect_conflict_with_global_name=getattr(
UniqueGlobalNames,
f"COLUMN_{primary_key_class.__name__.upper()}",
None,
),
module=Table.__module__,
target=UniqueGlobalNames.TABLE,
expect_conflict_with_global_name=UniqueGlobalNames.TABLE,
)
)
# Include the extra imports and definitions required for the
# primary column params.
pk_serialised_params: SerialisedParams = serialise_params(
params=value._meta.primary_key._meta.params
)

pk_serialised_params = _primary_key_params(value._meta.primary_key)
extra_imports.extend(pk_serialised_params.extra_imports)
extra_definitions.extend(pk_serialised_params.extra_definitions)

Expand Down
11 changes: 9 additions & 2 deletions piccolo/columns/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ def __repr__(self):

@dataclass
class ForeignKeyMeta:
references: t.Union[t.Type[Table], LazyTableReference]
references: t.Union[t.Type[Table], LazyTableReference, str]
on_delete: OnDelete
on_update: OnUpdate
target_column: t.Union[Column, str, None]
Expand All @@ -95,7 +95,14 @@ def resolved_references(self) -> t.Type[Table]:
from piccolo.table import Table

if isinstance(self.references, LazyTableReference):
return self.references.resolve()
if self.references.ready:
return self.references.resolve()
else:
raise ValueError(
f"The {self.references.table_class_name} table hasn't "
"fully initialised yet - please refactor your code so all "
"tables have imported before using them."
)
elif inspect.isclass(self.references) and issubclass(
self.references, Table
):
Expand Down
96 changes: 57 additions & 39 deletions piccolo/columns/column_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -1716,15 +1716,27 @@ class Musician(Table):

In certain situations, you may be unable to reference a ``Table`` class
if it causes a circular dependency. Try and avoid these by refactoring
your code. If unavoidable, you can specify a lazy reference. If the
``Table`` is defined in the same file:
your code. If unavoidable, you can specify a lazy reference using
:class:`LazyTableReference <piccolo.columns.reference.LazyTableReference>`.
If the ``Table`` is defined in the same file:

.. code-block:: python

from piccolo.columns.reference import LazyTableReference

class Band(Table):
manager = ForeignKey(references='Manager')
manager = ForeignKey(
references=LazyTableReference(
table_class_name="Manager",
module_path=__name__,
)
# Alternatively, Piccolo will interpret this string the
# same as above:
# references="Manager"
)

If the ``Table`` is defined in a Piccolo app:
Or if the ``Table`` is defined in a different file:
Python module:

.. code-block:: python

Expand All @@ -1733,12 +1745,15 @@ class Band(Table):
class Band(Table):
manager = ForeignKey(
references=LazyTableReference(
table_class_name="Manager", app_name="my_app",
table_class_name="Manager",
module_path="some_module.tables",
)
# Alternatively, Piccolo will interpret this string the
# same as above:
# references="some_module.tables.Manager"
)

If you aren't using Piccolo apps, you can specify a ``Table`` in any
Python module:
If the ``Table`` is defined in a Piccolo app, you can also use this:

.. code-block:: python

Expand All @@ -1747,12 +1762,8 @@ class Band(Table):
class Band(Table):
manager = ForeignKey(
references=LazyTableReference(
table_class_name="Manager",
module_path="some_module.tables",
table_class_name="Manager", app_name="my_app",
)
# Alternatively, Piccolo will interpret this string as
# the same as above:
# references="some_module.tables.Manager"
)

:param on_delete:
Expand Down Expand Up @@ -1887,19 +1898,38 @@ def __init__(
# The ``TableMetaclass``` sets the actual value for
# ``ForeignKeyMeta.references``, if the user passed in a string.
self._foreign_key_meta = ForeignKeyMeta(
references=Table if isinstance(references, str) else references,
references=references,
on_delete=on_delete,
on_update=on_update,
target_column=target_column,
)

def _setup(self, table_class: t.Type[Table]) -> ForeignKeySetupResponse:
def _on_ready(self):
"""
Once we know the table being referred to has been loaded, we call this
method.
"""
# Record the reverse relationship on the target table.
referenced_table = self._foreign_key_meta.resolved_references
if self not in referenced_table._meta._foreign_key_references:
referenced_table._meta._foreign_key_references.append(self)
# Allow columns on the referenced table to be accessed via
# auto completion.
self.set_proxy_columns()

def _setup(
self,
table_class: t.Type[Table],
loaded_table_classes: t.List[t.Type[Table]] = [],
) -> ForeignKeySetupResponse:
"""
This is called by the ``TableMetaclass``. A ``ForeignKey`` column can
only be completely setup once it's parent ``Table`` is known.

:param table_class:
The parent ``Table`` class for this column.
:param loaded_table_classes:
Any ``Table`` classes which have already been loaded.

"""
from piccolo.table import Table
Expand Down Expand Up @@ -1929,26 +1959,30 @@ def _setup(self, table_class: t.Type[Table]) -> ForeignKeySetupResponse:
module_path=module_path,
)

self._foreign_key_meta.references = references

is_lazy = isinstance(references, LazyTableReference)

if is_lazy:
# We should check if the referenced tables are already
# available.
if references.check_ready(
table_classes=loaded_table_classes
).was_changed:
self._on_ready()

is_table_class = inspect.isclass(references) and issubclass(
references, Table
)

if is_lazy or is_table_class:
self._foreign_key_meta.references = references
else:
if not (is_lazy or is_table_class):
raise ValueError(
"Error - ``references`` must be a ``Table`` subclass, or "
"a ``LazyTableReference`` instance."
)

if is_table_class:
# Record the reverse relationship on the target table.
references._meta._foreign_key_references.append(self)

# Allow columns on the referenced table to be accessed via
# auto completion.
self.set_proxy_columns()
self._on_ready()

return ForeignKeySetupResponse(is_lazy=is_lazy)

Expand Down Expand Up @@ -2084,22 +2118,6 @@ def __getattribute__(self, name: str) -> t.Union[Column, t.Any]:
case a copy is returned with an updated call_chain (which records the
joins required).
"""
# If the ForeignKey is using a lazy reference, we need to set the
# attributes here. Attributes starting with a double underscore are
# unlikely to be column names.
if not name.startswith("__"):
try:
_foreign_key_meta = object.__getattribute__(
self, "_foreign_key_meta"
)
except AttributeError:
pass
else:
if _foreign_key_meta.proxy_columns == [] and isinstance(
_foreign_key_meta.references, LazyTableReference
):
object.__getattribute__(self, "set_proxy_columns")()

try:
value = object.__getattribute__(self, name)
except AttributeError:
Expand Down
Loading