tsileo / little-boxes

Tiny ActivityPub framework written in Python, both database and server agnostic.
https://little-boxes.a4.io
ISC License
81 stars 7 forks source link

Move fetch_iri from Backend #5

Closed autogestion closed 6 years ago

autogestion commented 6 years ago

This method is not about backend and it brokes flow for async app Also, when posting to outbox there is no need to make external request like in BaseActivity._validate_person()

tsileo commented 6 years ago

Hi!

This method is about backend! Everything can be referenced by its IRI, so the "backend" need a way to dereference things (by fetching them), like the object of a Like.

But this specific method (fetch_iri) is supposed to be overidded by the backend, but I haven't written much documentation yet (example here: https://github.com/tsileo/microblog.pub/blob/master/activitypub.py#L96 ).

microblog.pub and little_boxes are not designed to be async from the ground-up, and I have no plans to do so for now. And in microblog.pub, I'm using Celery for taking care of spawning async tasks. I will give more thought about how we can possibly support async but I don't think there will be an easy way.

And as for the _validate_person calls, I agree that it's called too many times for now, that will be fixed once the framework mature a little bit.

autogestion commented 6 years ago

I guess I used wrong term. I meant that fetch_iri is not Database level, and by Backend I meant db. So what if split Backend class in two classes, one for interaction with db and one for manipulating with urls and ids In order not to break existing flow, it could be made in next way

class Network:
    def fetch_iri...

class DB:
    def outbox_new...

class Backend (Network, DB):
    pass

Generally, this Network class is pretty same as "Application" from #6 I can sort them out and make PR)

tsileo commented 6 years ago

I don't know, my points against #6 still applies.

It's not that simple, I feel like the "network" and the "DB" are too tightly coupled, you need to call network from the DB (like de-referencing a the object of a Like or a Follow) and the DB from the network (if you ask for a local collection, it should go through the DB and should not make HTTP query to itself). What are you thoughts on that?

I like it the way it is right now (as I said in #6), not sure my opinion will change. I don't say it's the best way to deal with ActivityPub (the "backend of side-effect" concept), but I like how the backend looks in microblog.pub, and I started another project (not published yet) where I feel the current API just fits.

tsileo commented 6 years ago

I thought more about your issue, I think I will try to move more stuff that are currently in BaseActivity to Outbox/Inbox, mainly the stuff that calls the DB. This way you should be able to use little_boxes just to parse the activity , do your async network stuff in overridded metohds (that yield stuff), for the rest (inbox_like...) you can have them do nothing. This way could sill use little_boxes, but you will have to implement "the side effect" (like posting an Accept in response to a Follow) yourself.

I will think more about this.

What do you think? I feel it could be a good compromise. (cc @dsblank )

dsblank commented 6 years ago

My goal is to (help) create a general Python module for all activitypub uses. This may not align with the goals here now, or ever. So, my comments may not be applicable. The main point is that a good, generic activitypub module should be easy for new developers to pick up and adapt for their uses. Issue tracker comments like @autogestion 's suggest that the current design is not necessarily easy to understand where the separations are.

The database definitely needs an abstraction layer, and should be a attribute of a backend (not a parent class). I don't know if it matters too much if network activity needs to be a separate class, or just a section of methods in the backend. In playing around with little-boxes, I have had to overwrite network methods for mock tests. If network connections were collected into a single class, that might make that easier for testing. I think it might make it easier for people adapting an activitypub module to have a Network class, and that is important to me.

tsileo commented 6 years ago

The current design is not documented that much, so that may be part of the issues.

I'm against the database abstraction because I don't see the point of it (except make things less flexible and more complicated).

And about the need to mock/overvwrite the network call, they are designed to be overridden, you don't want to call your own server for resolving a local collection. And it's pretty common to mock network calls in tests.

Anyway, there's still a lot to be done, but it works well for microblob.pub, which was my primary goal, and other people may find it useful too.

So I'm just giving up on other use cases for now and will focus on microblog.pub.

autogestion commented 6 years ago

@tsileo sorry for returning to this issue again, have some ideas and need to share. I understand that current structure is well for you, that's only ideas how it be could used widely in some future. That's not a request to implement it now

So, little-boxes could be divided into 3 unbound modules:

  1. object creator/parser: checks object structure and presence of the necessary keys on init. provides 2 mandatory attributes, one is rendered object, second - list of entitiy ids to validate.
  2. entities validator - takes list, sends requests to db or remote server. returns true if all valid or raise some kind of Validation error. User can use it's own validator
  3. cryptography - sign and verify requests

So it could be used for outbox post in next order:

And for inbox post:

Also little-boxes could provide same Inbox/Outbox classes, which runs unbound modules one by one. For plug-n-play use, if user have same technology stack and goals, as microblog

I'm using unbound utilities from little-boxes and I'm fine with it. That's just an ideas)

tsileo commented 6 years ago

Hey! No problem, thanks for sharing your ideas.

I will think about it, by the list of entity IDs, you mean the "actor IRI" and the "object IRI" (if it's not embedded) for example, right?

And by the way, feel free to join the Matrix room #microblogpub if you want to stay updated about the development of both microblog.pub and little-boxes!

Thanks!

autogestion commented 6 years ago

Yep, I mean the "actor IRI" and the "object IRI"

Thanks for invite)