Skip to content
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 more stream functions #24

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add more stream functions #24

wants to merge 3 commits into from

Conversation

stefan-mysten
Copy link
Collaborator

No description provided.

@stefan-mysten stefan-mysten self-assigned this Oct 1, 2024
Comment on lines 326 to 327
after: Option<String>,
before: Option<String>,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why require an owned string now?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I did like this because I need to pass the event filter in the streams calls, so I needed to clone it. Maybe I can just pass a reference.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bmwill this is what I came up with, what do you think?

@bmwill
Copy link
Collaborator

bmwill commented Oct 4, 2024

@stefan-mysten I wonder...this PR has quite a bit of repetitive code, would it be possible to generalize this and have a generic "stream" query function that can be reused everywhere? As in we have a function for issuing a generic graphql request, can we have something similar (with the constraint that we may have paging information that is standardized) that will be used as inputs into the query, we could then let anyone trivially run a pagable query as a stream

@stefan-mysten
Copy link
Collaborator Author

@bmwill Interesting idea, yes, I can think we can try that. I'll give it a go and see if I can make it work!

@stefan-mysten
Copy link
Collaborator Author

stefan-mysten commented Oct 7, 2024

@bmwill I looked into this over the weekend. I think it's doable, the only issue is how to "rebuild" the query with a new set of variables (because the cursor changes and we need to re-run the query to fetch the next page with a different after condition) programmatically. I stumbled a bit over this and couldn't find a clean and straightforward way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants