-
Notifications
You must be signed in to change notification settings - Fork 88
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
add relay support #641
add relay support #641
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, can you explain why you think a separate Relay actor is required, that's not SystemActor nor a standard Identity? From scanning through, it doesn't seem like it does anything that either of those couldn't be extended to do.
@@ -264,6 +263,7 @@ class Migration(migrations.Migration): | |||
("identity_edited", "Identity Edited"), | |||
("identity_deleted", "Identity Deleted"), | |||
("identity_created", "Identity Created"), | |||
("identity_moved", "Identity Moved"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll do this on main, it shouldn't need to be in here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure, reverted.
9f1dc90
to
4d646c9
Compare
@andrewgodwin I had the same idea as yours and was using system actor when started working on this. However, after digging thru a few relay implementations, I realize, in order to be compatible with them, the relay actor has to:
Changing these on system actor especially actor uri will cause problem for existing deployments, plus there might be other relay specific hacks as I might not cover all those relay implementation tricks. Hence I think the best approach is having them on a dedicated actor, not reuse system who has other duties. |
4d646c9
to
2dfbeee
Compare
The shared inbox thing we can do with system actor, but what software requires that the relay actor ends in '/relay'? That seems... strange. |
@andrewgodwin even more strange one: I guess most of these were probably built when there were only Mastodon and Pleroma. An alternative is, instead of LitePub (Pleroma) mode, we do handshake in Mastodon mode. That's doable and it was where I started, but I gave up bc that requires even more ugly hack in Takahe follow and inbox code flow. |
What sort of hack is required for "mastodon mode"? I'm curious, as it would seem less fragile than mixing the modes as is done currently, and reading your short description of it makes it sound easier (just send a follow at the start and then sling posts at the remote inbox as we get them) |
@andrewgodwin mastodon mode is an incomplete Follow flow, so instead of reuse the current Follow code, extra code has to be added to
|
I might say that, instead of doing special things with Identities, we should just have a new Relay model that does this instead? It can have a "unsent"/"sent" state to handle this follow, and it would made the admin code cleaner as well. |
db34930
to
7629395
Compare
I can't say I'm very convinced. Conceptually, a relay is not much different from a real Yes, there are some hacks but that's quite minimum, only to initialize the local relay identity with a predetermined actor_uri and username; the admin view is just to manage follower of an identity, if we can find existing view/template to reuse, it's not even necessary; or if the concern is mainly the state display, we can make it a property of Follow? btw I just removed some unnecessary code accidentally left over from my previous approaches so the total code change is even less now. On the other hand, a RelayState + Relay model will add/duplicate more code, and not much additional value, which feels a bit over kill to me, |
Well, Takahē will accept anything posted to its inbox URLs without even needing a follow object for it - internally, it treats every inbound message the same (shared inbox or personal inbox) and works out its own fanout, so we really don't need to even know about the relay for inbound content. Thus, that means the only thing we'd be using Identity for is the follow accept/reject logic; the message acceptance doesn't need an identity (it can use either the shared inbox URI, or we can invent one with relay in the name that goes to the same place if we want), and sending every local message out to a relay has to be special code anyway that adds a fanout for every relay in the targets calculation code. (I have been planning to refactor out some of the actual ActivityPub parsing/formatting code a little, too, which might close the gap in a situation like this; there's a bit too much of it in the models right now) |
That's not only for the logic, but also to store property(target relay's actor uri and inbox uri) and state, so we don't have to add an extra Relay model to store their property and states. It might be just personal aesthetics but I really don't prefer the redundant code in the alternative approach of extra Relay model which provides some clarity but very limited IMHO. However, I think it's important for Takahe to support the relay feature, so if you really still prefer the approach of extra Relay model after considering the discussion so far, I can put together a different PR. |
7629395
to
eac6158
Compare
eac6158
to
7629395
Compare
reimplemented in #686 |
Protocol
from
users/models/relay_actor.py
:The mixed mode is chosen to minimize necessary hacks to the current code and data model
Test
The PR has been tested with instances running the following relay implementations
Usage
Relay can be added in admin UI (screen shot below), or run the following in
python manage.py shell
:be aware: large relay may take minutes or longer to respond your subscribing request, and once it does, large volume of activities will be posted to your server at once.