golgi-ssb
/
golgi
4
5
Fork 0
Code Issues 11 Pull Requests 1 Projects 1 Releases Wiki Activity

Add methods for following, blocking and looking up peers #18

Merged
glyph merged 8 commits from friends-requests into sbot-connection 2022-02-04 08:31:02 +00:00
Conversation 4 Commits 8 Files Changed 3 +228 -25
notplants commented 2022-01-15 15:37:59 +00:00
Owner
Copy Link
No description provided.
notplants added 3 commits 2022-01-15 15:38:00 +00:00
notplants commented 2022-01-15 15:47:26 +00:00
Author
Owner
Copy Link

I added an example examples/ssb-friends.rs which probably shows the state of things best.

The strangest bug I found, is that go-sbot friends hops is not finding the most recent contact message... but it finds all contact messages before that just fine.

This bug is seen for both friends hops, as well as friends isFollowing and friends isBlocking.

This means the first time you run the ssb-friends example it will return the incorrect values. But if you re-run it, every time after that it will return the correct values.

Here are the relevant functions that were added which make muxrpc calls:

set_relationship(&mut self, contact: &str, following: bool, blocking: bool)

friends_is_following(
        &mut self,
        src_id: &str,
        dest_id: &str,
    )
    
friends_is_blocking(
        &mut self,
        src_id: &str,
        dest_id: &str,
    )

friends_hops(
        &mut self,
        opts: FriendsHopsOpts,
    )

The other functions added are syntactic sugar on top of these functions.

One other important possible bug I found in go-sbot. friends hops does not seem to take into account the reverse parameter (documented here https://github.com/ssbc/ssb-friends) which is used to get your followers instead of your follows. Currently friends hops returns your follows regardless what you pass here, and I haven't thought of any way to get your followers.

I wrote all of these different functions at once, so we could see the whole state of the friends API and find if there are any workarounds. I believe if we fixed those two bugs mentioned above in go-sbot, then we could get all the info we need for PeachPub.

I also listed a couple other possible issues I found in this doc (https://server.commoninternet.net/pad/p/sbot_issues) but I don't think they are blocking issues.

I added an example examples/ssb-friends.rs which probably shows the state of things best. The strangest bug I found, is that go-sbot friends hops is not finding the most recent contact message... but it finds all contact messages before that just fine. This bug is seen for both friends hops, as well as friends isFollowing and friends isBlocking. This means the first time you run the ssb-friends example it will return the incorrect values. But if you re-run it, every time after that it will return the correct values. Here are the relevant functions that were added which make muxrpc calls: ``` set_relationship(&mut self, contact: &str, following: bool, blocking: bool) friends_is_following( &mut self, src_id: &str, dest_id: &str, ) friends_is_blocking( &mut self, src_id: &str, dest_id: &str, ) friends_hops( &mut self, opts: FriendsHopsOpts, ) ``` The other functions added are syntactic sugar on top of these functions. One other important possible bug I found in go-sbot. friends hops does not seem to take into account the reverse parameter (documented here https://github.com/ssbc/ssb-friends) which is used to get your followers instead of your follows. Currently friends hops returns your follows regardless what you pass here, and I haven't thought of any way to get your followers. I wrote all of these different functions at once, so we could see the whole state of the friends API and find if there are any workarounds. I believe if we fixed those two bugs mentioned above in go-sbot, then we could get all the info we need for PeachPub. I also listed a couple other possible issues I found in this doc (https://server.commoninternet.net/pad/p/sbot_issues) but I don't think they are blocking issues.
notplants commented 2022-01-15 15:50:41 +00:00
Author
Owner
Copy Link

Here is a PR with the additions to kuska that make this golgi code functional:
https://github.com/mhfowler/kuska-ssb/pull/1

Here is a PR with the additions to kuska that make this golgi code functional: https://github.com/mhfowler/kuska-ssb/pull/1
notplants commented 2022-01-15 16:05:53 +00:00
Author
Owner
Copy Link

Also added a git hook for cargo fmt, which I'm now using, and hopefully reduces whitespace clutter

Also added a git hook for cargo fmt, which I'm now using, and hopefully reduces whitespace clutter
notplants added 1 commit 2022-01-16 13:55:13 +00:00
glyph reviewed 2022-01-17 10:00:48 +00:00
glyph left a comment
Owner
Copy Link

@notplants

I've made a couple of comments. There are some things we need to consider before merging, I think.

I also agree with the comment @cblgh made on cabal:

i think in some cases, for peachpub, we will have to be clever with what is there instead of trying to fit everything that is not there into it

@notplants I've made a couple of comments. There are some things we need to consider before merging, I think. I also agree with the comment @cblgh made on cabal: > i think in some cases, for peachpub, we will have to be clever with what is there instead of trying to fit everything that is not there into it
src/sbot.rs
@ -183,0 +215,4 @@
/// if state is false, then sets the following status to false (unfollow)
/// TODO: currently this method is not working (seems to be missing from go-sbot)
/// as a workaround, we can use set_relationship instead just fine
pub async fn friends_follow(
glyph commented 2022-01-17 09:56:01 +00:00
Owner
Copy Link

I'm a bit puzzled by the utility of this method. Could you help me understand? Why not just have two functions: follow() and unfollow()?

I'm a bit puzzled by the utility of this method. Could you help me understand? Why not just have two functions: `follow()` and `unfollow()`?
notplants commented 2022-01-17 15:05:24 +00:00
Author
Owner
Copy Link

@glyph friends.follow is one of the muxrpc methods defined in https://github.com/ssbc/ssb-friends. I thought we could definite it here in the same way so there is some symmetry between the golgi api and the js side.

It seems that friends.follow is not currently implemented in go-sbot anyway,
so I would actually just remove this function for now anyway,
and use the set_relationsip function I wrote above (and we can also write follow and unfollow methods that call set_relationship).

In the future, in an ideal world, I think it could be a good idea for golgi to intentionally implement all of the muxrpc methods as part of its API, in the same way as they are in javascript, to make documentation clear and uniform, between both stacks,

even if golgi also then implements additional convenience methods that do things in other ways. But since go-sbot doesnt seem to yet implement all the same muxrpc calls as the js side, then this interface symmetry is not achievable for now anyway

@glyph friends.follow is one of the muxrpc methods defined in https://github.com/ssbc/ssb-friends. I thought we could definite it here in the same way so there is some symmetry between the golgi api and the js side. It seems that friends.follow is not currently implemented in go-sbot anyway, so I would actually just remove this function for now anyway, and use the set_relationsip function I wrote above (and we can also write follow and unfollow methods that call set_relationship). In the future, in an ideal world, I think it could be a good idea for golgi to intentionally implement all of the muxrpc methods as part of its API, in the same way as they are in javascript, to make documentation clear and uniform, between both stacks, even if golgi also then implements additional convenience methods that do things in other ways. But since go-sbot doesnt seem to yet implement all the same muxrpc calls as the js side, then this interface symmetry is not achievable for now anyway
glyph commented 2022-01-17 15:29:09 +00:00
Owner
Copy Link

Thanks for explaining.

so I would actually just remove this function for now anyway,
and use the set_relationsip function I wrote above (and we can also write follow and unfollow methods that call set_relationship).

This sounds great to me. I really like your set_relationship function. It's so much clearer to me than friends.follow.

In the future, in an ideal world, I think it could be a good idea for golgi to intentionally implement all of the muxrpc methods as part of its API, in the same way as they are in javascript, to make documentation clear and uniform, between both stacks,

I'd love to discuss this in detail another time because I think it's a very important design decision. My brief opinion is that there are some ugly parts of the JS API that I really don't want to emulate just to achieve a 1:1 mapping. I'd rather have a beautiful, clear and consistent golgi API that deviated from JS in places.

One option might be to ask arj, staltz and mix for their opinion ("which of these methods are necessary in your opinion and which are best left out?"), since they're the ones with the most experience of the JS stack.

Thanks for explaining. > so I would actually just remove this function for now anyway, and use the set_relationsip function I wrote above (and we can also write follow and unfollow methods that call set_relationship). This sounds great to me. I really like your `set_relationship` function. It's so much clearer to me than `friends.follow`. > In the future, in an ideal world, I think it could be a good idea for golgi to intentionally implement all of the muxrpc methods as part of its API, in the same way as they are in javascript, to make documentation clear and uniform, between both stacks, I'd love to discuss this in detail another time because I think it's a very important design decision. My brief opinion is that there are some ugly parts of the JS API that I really don't want to emulate just to achieve a 1:1 mapping. I'd rather have a beautiful, clear and consistent golgi API that deviated from JS in places. One option might be to ask arj, staltz and mix for their opinion ("which of these methods are necessary in your opinion and which are best left out?"), since they're the ones with the most experience of the JS stack.
notplants commented 2022-01-17 15:52:36 +00:00
Author
Owner
Copy Link

hmm interesting, could see either way ~

I guess its mostly a question of exclusion

A - I could imagine a muxrpc file in golgi which simply includes a 1:1 golgi function for each muxrpc function,
even if most rust applications on top of golgi dont actually use all those calls

B - could intentionally leave out functions like friends.follow which we decide golgi doesn't want to support

hmm interesting, could see either way ~ I guess its mostly a question of exclusion A - I could imagine a muxrpc file in golgi which simply includes a 1:1 golgi function for each muxrpc function, even if most rust applications on top of golgi dont actually use all those calls B - could intentionally leave out functions like friends.follow which we decide golgi doesn't want to support
cblgh commented 2022-01-18 14:02:11 +00:00
Member
Copy Link

re friends.follow not existing, just for reference: here's how sbotcli handles it: https://github.com/cryptoscope/ssb/blob/master/cmd/sbotcli/publish.go#L229

(you publish a contact message with following: true)

re friends.follow not existing, just for reference: here's how sbotcli handles it: https://github.com/cryptoscope/ssb/blob/master/cmd/sbotcli/publish.go#L229 (you publish a contact message with `following: true`)
❤️ 1
src/sbot.rs
@ -183,0 +286,4 @@
}
// Gets a Vec<String> where each element is a peer who follows you
/// TODO: currently this method is not working
glyph commented 2022-01-17 09:58:28 +00:00
Owner
Copy Link

As far as I can tell by looking at the Go codecase, the reverse parameter is not supported for this RPC call. I think it might be best to keep such calls out of the public API of golgi to avoid any confusion, at least until some future time when either A) we learn that the parameter is actually supported but not working for some reason, or B) parameter support is added.

As far as I can tell by looking at the Go codecase, the `reverse` parameter is not supported for this RPC call. I think it might be best to keep such calls out of the public API of golgi to avoid any confusion, at least until some future time when either A) we learn that the parameter is actually supported but not working for some reason, or B) parameter support is added.
notplants commented 2022-01-17 15:12:28 +00:00
Author
Owner
Copy Link

that sounds like a good plan - in this PR I was mostly just trying to document everything I tried so we could see the state of what's missing.

the missing reverse parameter is a particularly big missing feature, since I'm not sure how we can look up someone's followers without it.

Maybe I could modify this function to return an empty list for now,
with a todo next to it,
and then we could still call this function in the PeachPub UI for the routes that need it, knowing that eventually it will be implemented,
but for now it cannot be due to a limitation of go-sbot.

Could also remove the function entirely, but it does seem like this is pretty much the function we would want, at some point.

that sounds like a good plan - in this PR I was mostly just trying to document everything I tried so we could see the state of what's missing. the missing reverse parameter is a particularly big missing feature, since I'm not sure how we can look up someone's followers without it. Maybe I could modify this function to return an empty list for now, with a todo next to it, and then we could still call this function in the PeachPub UI for the routes that need it, knowing that eventually it will be implemented, but for now it cannot be due to a limitation of go-sbot. Could also remove the function entirely, but it does seem like this is pretty much the function we would want, at some point.
glyph commented 2022-01-17 15:15:20 +00:00
Owner
Copy Link

Oh yeah I just meant keeping it out of the public API (no pub before fn...) so that it's still all there but not available to a library user.

Oh yeah I just meant keeping it out of the public API (no `pub` before `fn...`) so that it's still all there but not available to a library user.
notplants commented 2022-01-17 15:56:49 +00:00
Author
Owner
Copy Link

hmm since we actually will want to use this function to write the Peers route in peach-web, we would need it to be pub for that. But for now I'll removed the pub.

hmm since we actually will want to use this function to write the Peers route in peach-web, we would need it to be pub for that. But for now I'll removed the pub.
notplants added 2 commits 2022-01-17 15:57:39 +00:00
glyph added 2 commits 2022-02-04 08:26:29 +00:00
glyph commented 2022-02-04 08:30:42 +00:00
Owner
Copy Link

I've updated this PR to match the friends RPC API in kuska (PR#21) which uses FriendsHops instead of FriendsHopsOptions and introduces the RelationshipQuery struct to unite source and dest.

I'm going ahead with the merge 🤘

I've updated this PR to match the friends RPC API in kuska ([PR#21](https://github.com/Kuska-ssb/ssb/pull/21)) which uses `FriendsHops` instead of `FriendsHopsOptions` and introduces the `RelationshipQuery` `struct` to unite `source` and `dest`. I'm going ahead with the merge 🤘
glyph merged commit ec348d57ee into sbot-connection 2022-02-04 08:31:02 +00:00
glyph deleted branch friends-requests 2022-02-04 08:31:02 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
bug

Something is not working

  
duplicate

This issue or pull request already exists

  
enhancement

New feature

  
help wanted

Need some help

  
invalid

Something is wrong

  
question

More information is needed

  
wontfix

This won't be fixed

No Label
bug
duplicate
enhancement
help wanted
invalid
question
wontfix
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: golgi-ssb/golgi#18
No description provided.
Delete Branch "friends-requests"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?