Implement get_about_message_stream #12

notplants · 2022-01-05T20:12:46Z

notplants commented

2022-01-05 20:12:46 +00:00

This PR changes the get_subset and get_about_messages logic to use the latest stream code.

While working on this these other small changes slipped into this PR:

cleanup whoami to return just a String of the actual id (not a jsonified string)
cleanup print statements in examples

The get_subset_stream and get_about_messages logic can be completed and further refined once the get_subset regression is fixed and the latest changes are merged into kuska (allowing for passing descending flag).

But cool to see its pretty easy to work with these Stream objects for our different needs.

You could review this now, and we could merge it in, or I could add the other changes mentioned above, pending the go-sbot + kuska changes.

I think I will hold off on further golgi work until those changes are in.

cc @glyph

This PR changes the get_subset and get_about_messages logic to use the latest stream code. While working on this these other small changes slipped into this PR: - cleanup whoami to return just a String of the actual id (not a jsonified string) - cleanup print statements in examples The get_subset_stream and get_about_messages logic can be completed and further refined once the get_subset regression is fixed and the latest changes are merged into kuska (allowing for passing descending flag). But cool to see its pretty easy to work with these Stream objects for our different needs. You could review this now, and we could merge it in, or I could add the other changes mentioned above, pending the go-sbot + kuska changes. I think I will hold off on further golgi work until those changes are in. cc @glyph

notplants added 1 commit 2022-01-05 20:12:47 +00:00

Implement get_about_message_stream aa60a6e136

notplants requested review from glyph 2022-01-05 20:48:38 +00:00

notplants changed target branch from async-stream to main

2022-01-06 15:32:17 +00:00

notplants added 1 commit 2022-01-12 17:32:30 +00:00

Change get_latest_about_message to use streams a3852d89c9

notplants added 1 commit 2022-01-12 17:37:21 +00:00

Change to use and_then 6c6413c1b4

notplants commented

2022-01-12 17:38:07 +00:00

latest commit improved the code by using and_then

An explanation (from here: https://riptutorial.com/rust/example/9668/using-option-with-map-and-and-then)
// 'and_then' is very similar to 'map', but allows us to pass a
// function which returns an Option type itself. To ensure that we
// don't end up with Option<Option>, 'and_then' flattens the
// result (in other languages, 'and_then' is also known as 'flatmap').

latest commit improved the code by using and_then An explanation (from here: https://riptutorial.com/rust/example/9668/using-option-with-map-and-and-then) // 'and_then' is very similar to 'map', but allows us to pass a // function which returns an Option type itself. To ensure that we // don't end up with Option<Option<i32>>, 'and_then' flattens the // result (in other languages, 'and_then' is also known as 'flatmap').

👍 1

notplants added 1 commit 2022-01-12 17:43:05 +00:00

Use ok_or_else 615431496b

notplants commented

2022-01-12 17:44:02 +00:00

The latest commit, changing to use ok_or_else was a suggestion by clippy (I believe the use of a closure makes the allocation lazy, only run if needed, which clippy prefers)

👍 1

notplants commented

2022-01-12 17:47:26 +00:00

This PR now changes get_latest_about_message to use a stream, and to use find, instead of first converting the stream to a vector (so theoretically this will be more efficient, if there were many about messages, although the difference may in practice be negligble).

Changing to use a stream meant the filtering and mapping you did needed to be slightly reorganized. I had to use a match statement again, but atleast not nested match statements. Maybe there are further style optimizations possible, but I wasn't sure how to get it cleaner than this.

Ready for review @glyph

This PR now changes get_latest_about_message to use a stream, and to use find, instead of first converting the stream to a vector (so theoretically this will be more efficient, if there were many about messages, although the difference may in practice be negligble). Changing to use a stream meant the filtering and mapping you did needed to be slightly reorganized. I had to use a match statement again, but atleast not nested match statements. Maybe there are further style optimizations possible, but I wasn't sure how to get it cleaner than this. Ready for review @glyph

notplants added 1 commit 2022-01-12 20:01:52 +00:00

Convert result to option 0addd8dc47

notplants commented

2022-01-12 20:02:33 +00:00

Added one more commit that gets rid of the match statement

notplants added 1 commit 2022-01-12 20:23:02 +00:00

Chang get_subset_stream to return SsbMessageValue instead of SsbMessageKVT 1879341499

notplants commented

2022-01-12 20:23:42 +00:00

hmm for some reason get_subset started returning SsbMessageValue instead of SsbMessageKVT ... do you know why this might be?

I updated the code to expect this now and its testing and working

hmm for some reason get_subset started returning SsbMessageValue instead of SsbMessageKVT ... do you know why this might be? I updated the code to expect this now and its testing and working

glyph commented

2022-01-13 07:12:44 +00:00

@notplants

hmm for some reason get_subset started returning SsbMessageValue instead of SsbMessageKVT ... do you know why this might be?

I updated the code to expect this now and its testing and working

Yes, thankfully I know why this is. I noticed it when reading your example code last night.

In get_about_message_stream() you defined None for the keys field of the options struct:

let query_options = SubsetQueryOptions {
    descending: Some(true),
    keys: None,
    page_limit: None,
};

keys controls the form of the returned message. If it's true, you get a message as a KVT. If it's false, you get it as a message value. So you could simply change L252 in src/sbot.rs to be keys: Some(true), and you will then get back messages as KVTs.

@notplants >hmm for some reason get_subset started returning SsbMessageValue instead of SsbMessageKVT ... do you know why this might be? > >I updated the code to expect this now and its testing and working Yes, thankfully I know why this is. I noticed it when reading your example code last night. In `get_about_message_stream()` you defined `None` for the `keys` field of the options `struct`: ```rust let query_options = SubsetQueryOptions { descending: Some(true), keys: None, page_limit: None, }; ``` `keys` controls the form of the returned message. If it's true, you get a message as a KVT. If it's false, you get it as a message value. So you could simply change L252 in `src/sbot.rs` to be `keys: Some(true),` and you will then get back messages as KVTs.

notplants commented

2022-01-13 14:00:16 +00:00

@glyph aw cool, glad to have an explanation. I think I will keep it returning SsbMessageValue since that's what we need anyway, but good to know its not just a random ghost in the sbot

glyph approved these changes 2022-01-13 14:57:19 +00:00

glyph left a comment

LGTM. Great work! 🟢

notplants merged commit 88c198c0d8 into main

2022-01-13 15:05:03 +00:00

notplants referenced this issue from a commit

2022-01-13 15:05:04 +00:00

Merge pull request 'Implement get_about_message_stream' (#12) from about-stream into main

notplants deleted branch about-stream

2022-01-13 15:05:54 +00:00

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: golgi-ssb/golgi#12