Skip to content
This repository was archived by the owner on Sep 4, 2025. It is now read-only.

AddInternalAsync refactor and explicit RemoveAsync#92

Merged
emanuelgaspar merged 5 commits intodevfrom
feature/add-internal-async-refactor
May 24, 2023
Merged

AddInternalAsync refactor and explicit RemoveAsync#92
emanuelgaspar merged 5 commits intodevfrom
feature/add-internal-async-refactor

Conversation

@emanuelgaspar
Copy link
Copy Markdown
Contributor

@emanuelgaspar emanuelgaspar commented May 23, 2023

The detection of trying to SubRemove subcriptions when calling AddInternalAsync was difficult to understand due to its use of JsonDocument.Parse(subscription.Topic).

This PR introduces a strongly-typed deserialization of the topic, which allows us to work on the array of subs with clean code.

New RemoveAsync(TopicSubscription subscription) method added to ICryptoCompareWebsocketHandler.
There's one from the base interface in websockets which takes an IList of subscriptions, but doesn't do what we need in market-data.

Also bumped some nuget/actions versions.

@emanuelgaspar emanuelgaspar marked this pull request as ready for review May 23, 2023 11:57
@emanuelgaspar emanuelgaspar changed the title AddInternalAsync refactor when removing subscriptions AddInternalAsync refactor and explicit RemoveAsync May 23, 2023
Comment thread src/Trakx.CryptoCompare.ApiClient.Websocket/CryptoCompareWebsocketHandler.cs Outdated
Comment thread src/Trakx.CryptoCompare.ApiClient.Websocket/CryptoCompareWebsocketHandler.cs Outdated
@emanuelgaspar emanuelgaspar requested a review from goneneren May 24, 2023 12:18
@emanuelgaspar emanuelgaspar merged commit 9d3c48a into dev May 24, 2023
@emanuelgaspar emanuelgaspar deleted the feature/add-internal-async-refactor branch May 24, 2023 12:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants