anvilistas / anvil-labs

MIT License
9 stars 3 forks source link

Add _persist module to kompot #109

Closed meatballs closed 1 year ago

meatballs commented 1 year ago

As per our recent discussion, this is the first attempt to use data table rows for server-client serialization and kompot registered classes for client-server.

meatballs commented 1 year ago

Sorry, I should have made this a draft. It was just to show WIP, but thanks for the various catches!

I'm not entirely happy with the API yet and and need to sort out passing the row for update calls so the client side caching happens automatically.

meatballs commented 1 year ago

I have an app using this that's working now, so it's ready for a proper review. I'll need to knock up some docs for it at some point!

meatballs commented 1 year ago

What about attributes that are dates and date times? Those would then need handling.

I used kompot so that's done behind the scenes.

s-cork commented 1 year ago

anvil's builtin serialization from server <-> client already knows how to do dates/datetimes

meatballs commented 1 year ago

Really?!!!! I missed that one coming in!

meatballs commented 1 year ago

In that case, I think I agree. I'll rip it apart...

meatballs commented 1 year ago

This should now make the API for LinkedAttribute look like:

@row_backed_class
class Book:
    author_name = LinkedAttribute(linked_column="author", linked_attr="name")
s-cork commented 1 year ago

yeah: re datetimes - https://anvil.works/docs/server#valid-arguments-and-return-values

Anvil can only transfer certain data types as the arguments or return values of @anvil.server.callable functions. We call these portable types, and these are:

Strings Numbers Lists Booleans (True and False) Dicts (with strings as keys) datetime.date and datetime.datetime objects None ...

meatballs commented 1 year ago

I never noticed dates and datetimes getting added to that list. I've always had to sort those out myself. Nice!

meatballs commented 1 year ago

At the moment, the Dispatcher instance is an attribute of a Store instance. That means I often have to refer to instance._store._dispatcher

How about, instead, moving it to the main instance itself. e.g. a Book instance would have both a _store and a _dispatcher attribute. Does that feel cleaner to you?

s-cork commented 1 year ago

At the moment, the Dispatcher instance is an attribute of a Store instance. That means I often have to refer to instance._store._dispatcher

How about, instead, moving it to the main instance itself. e.g. a Book instance would have both a _store and a _dispatcher attribute. Does that feel cleaner to you?

Yeah maybe, but I don't mind the current implementation

meatballs commented 1 year ago

I'm rewriting this as a separate module, so I'll close this PR and submit a new one once it's ready.