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

v5 Request - Adopt Async/Task based requests #100

Open
grofit opened this issue Sep 6, 2022 · 6 comments
Open

v5 Request - Adopt Async/Task based requests #100

grofit opened this issue Sep 6, 2022 · 6 comments

Comments

@grofit
Copy link
Contributor

grofit commented Sep 6, 2022

Issue Type

  • Feature Request

Describe the issue
Currently all calls to do requests with OBS are synchronous not making use of Task (and potentially async), given there has been a large push to move towards this paradigm by a lot of vendors who can do potentially long running tasks, i.e IO, Network, API, DB calls etc it would be good if we could provide async compatible requests.

Versions
OBS Version: N/A
OBS WebSocket Version: 5.x
OBS WebSocket Dotnet (this library) Version: 5.x
OS: N/A

Additional context
There was a PR offered for the v4 branch which included these changes which allowed for mainly non blocking requests to OBS which helps greatly with UI facing applications, unfortunately that PR never made it through, but it would be great if the same approaches could be rolled into the v5 branch.

@BarRaider
Copy link
Owner

BarRaider commented Sep 6, 2022

Overall I'm fine moving towards async/await however don't have the bandwidth for this.
If you want to make a targeted PR, can look into it

@grofit
Copy link
Contributor Author

grofit commented Sep 6, 2022

Thats understandable, at the moment I am still on the v4 version but at some point would want to move over to v5 so if I have time and you are open to it I will submit a PR for it as @Zingabopp has already done most of the legwork for this in the v4 branch, its just transposing their handling of it over and changing function sigs.

@BarRaider
Copy link
Owner

Just make sure it doesn't turn to be too big of PR

@grofit
Copy link
Contributor Author

grofit commented Sep 6, 2022

However you spin it, bare minimum its going to need to change the signatures of almost all request methods from T Whatever() to Task<T> Whatever() and all the corresponding methods related to it as well as cancellation token parts as well.

So just to make sure we are on the same page this will ultimately have cascading changes everywhere, but the PR (assuming im the one who does it) will JUST be related to this issue.

@BarRaider
Copy link
Owner

👍

@jeiea
Copy link

jeiea commented Oct 18, 2022

How about this?

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

No branches or pull requests

3 participants