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

AsyncSeq implementation for DynamoDB #16

Closed
wants to merge 1 commit into from

Conversation

rtkelly13
Copy link

Possible implementation for - #14

I think this is a useful addition, might need some work. Maybe splitting into a separate package, couldn't be done due to AttributeWriter being internal.

Had to bump up FSharp.Core version to support it.

@samritchie
Copy link
Collaborator

@rtkelly13 I’m working on pagination at the moment, once that’s done it will make adding a separate package considerably simpler, if that’s the best approach.

I don’t have a good feel for how widespread/popular AsyncSeq is, I’m not opposed to adding to this to the core library if it’s the best/most logical addition, but if everyone uses different reactive or async libraries then going separate may be a better idea.

We’d need tests & some examples in the readme.

@dsyme
Copy link

dsyme commented Apr 30, 2020

If I understand rightly this is the situation.

  1. There is an IAsyncEnumerable in the dotnet libraries these days. That should be the standard abstraction to implement.

  2. AsyncSeq should be updated to implement that.

  3. It is a great way to provide implementations of that abstraction in application code

  4. In library code the extra dependency may be an issue. There may be other implementation techniques around that don't induce a dependency, or you could roll your own internally.

@samritchie
Copy link
Collaborator

@dsyme would you expect the other async methods to be migrated to Task/ValueTask at the same time, or would you see just IAsyncEnumerable just being used in this specific case? I’m a bit wary of shipping Task-based and FSharpAsync-based methods in the same library.

@rtkelly13
Copy link
Author

I wasn't even aware of IAsyncEnumerable around the time I was doing this but yes that seems like a very good approach to take.
I'll see if I can dig out the code I originally pushed this PR for as I think I created a neater wrapper in the end outside of here. I was migrating data from one DynamoDB table to another and needed it
I think AsyncSeq is a great lib but yes I did bump up against the lack of Task support, this may have changed since.

@bartelink
Copy link
Member

FYI https://github.com/fsprojects/FSharp.Control.TaskSeq is available now - NOTE 0.3.0 has a bug in the impl of use, use! and finally, so need to use 0.4.0-alpha or later (Equinox.DynamoStore uses this lib with TaskSeq)

@samritchie
Copy link
Collaborator

I’ll close this PR as we’re more likely to use TaskSeq now to accomplish this - will continue tracking in #14

@samritchie samritchie closed this Jul 4, 2023
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.

4 participants