is method overriding supported?

[trondhindenes]

It's not clear to me if this is supported:

class Customer(Table):
    email: str = Varchar(null=True, default=None)
    first_name: str = Varchar(null=True, default=None)
    last_name: str = Varchar(null=True, default=None)

    def save(
            self, columns: t.Optional[t.List[t.Union[Column, str]]] = None
    ) -> t.Union[Insert, Update]:
        if not self.first_name:
            self.first_name = "Bobba"
        return super().save(columns)

it would be good to have some documentation around what's ok and not ok in terms of overriding methods, and which methods to typically override (save, update, delete etc)

if these overrides are supported, it would be very valuable to have async-enabled overrides (a typical use case would be to be able to fire a http request upon database object creation)

[dantownsend]

@trondhindenes Yes, good idea - it should be in the docs.

You can override the save method as you've done there. It should work in most other situations too.

[trondhindenes]

thanks, good to know! I couldn't find a way to override with an async, is there a hook for that?

[dantownsend]

If you override save, it will work for the sync and async use case.

await Customer(first_name="Bob").save()

# Or
Customer(first_name="Bob").save().run_sync()

But lets say you wanted to run an async query within your save method, that's a bit trickier. In that case you might be better off creating a separate method.

class Customer(Table):
    ...
    async def custom_save(self, columns):
        await some_method()
        return await self.save(columns)

await Customer(first_name="Bob").custom_save()

[trondhindenes]

gotcha, thanks. It would be great to have the ability to run code (async) in the context of save/delete/update methods automatically. I believe there was another issue and incomplete pr attempting to implement that.

[dantownsend]

Yeah, there was some discussion a while back about having signals, similar to Django (except the signals could be async).

Signals are quite tricky - they have valid use cases, but can make the control flow of an application harder to follow.

I'm not averse to adding something like signals, but some thought needs to go into the API. There's the Django approach where it's completely decoupled - so you can add signal handlers anywhere in the codebase:

@receiver(pre_save, sender=Customer)
def my_handler(sender, **kwargs):
    ...

Or there's something more explicit like:

class Customer(Table, pre_save=[my_function], post_save=[my_other_function]):
    ...

[trondhindenes]

yup, I have to admit we keep comparing piccolo/fastapi with Django since we run a bit of each.

we avoid django signals unless we have no other choice. Still, having the ability to just call Customer.save() in django and know that save hooks are handled (with the exception of cascading deletes) is very nice.

I'm not necessarily looking for signals - I don't need "fire and forget" functionality.

Simply being able to do

class Customer(Table):
    async def save(self):
        await do_stuff(self.email)

would be very helpful, especially in combination with piccolo-admin/piccolo-api since those libraries leave us with a little less control of the underlying codebase.

[dantownsend]

@trondhindenes Yes, I totally agree. I feel the need with Piccolo Admin / Piccolo API as well - being able to hook into it without overriding stuff would be very useful.

[trondhindenes]

my first idea was to imply have save() etc pass on a list of async functions into run() which would then execute them. But these hook functions probably need to be able to mutate the self, which would invalidate the "query". Tricky.

the code is a bit above my paygrade so I'm afraid I don't have much to contribute here other than cul-de-sacs :-(