Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

implement session handling for unsubscribe in subxt-client #242

Merged
merged 3 commits into from
Mar 9, 2021

Conversation

gregdhill
Copy link
Contributor

Signed-off-by: Gregory Hill gregorydhill@outlook.com

@@ -102,12 +111,19 @@ impl SubxtClient {
pub fn new(mut task_manager: TaskManager, rpc: RpcHandlers) -> Self {
let (to_back, from_front) = mpsc::channel(DEFAULT_CHANNEL_SIZE);

let request_id = Arc::new(RwLock::new(u64::MIN));
Copy link
Member

@niklasad1 niklasad1 Mar 9, 2021

Choose a reason for hiding this comment

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

I don't understand why you instantiate these here, inside task::spawn should work?

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess because the main closure is for each FrontToBack message. That said the context could be expanded although that is already an extremely indented block of code.

@@ -102,12 +111,19 @@ impl SubxtClient {
pub fn new(mut task_manager: TaskManager, rpc: RpcHandlers) -> Self {
let (to_back, from_front) = mpsc::channel(DEFAULT_CHANNEL_SIZE);

let request_id = Arc::new(RwLock::new(u64::MIN));
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess because the main closure is for each FrontToBack message. That said the context could be expanded although that is already an extremely indented block of code.

@gregdhill gregdhill force-pushed the subxt-client-unsubscribe branch from a978a4d to e92383d Compare March 9, 2021 17:13
Copy link
Contributor

@ascjones ascjones left a comment

Choose a reason for hiding this comment

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

LGTM

@ascjones
Copy link
Contributor

ascjones commented Mar 9, 2021

Not sure whether to merge this or not, the CI is failing. Says that we should merge this first here: #243.

Where will the build errors with FrontToBack be fixed @gregdhill?

gregdhill and others added 2 commits March 9, 2021 17:43
@gregdhill
Copy link
Contributor Author

Cargo seems to default to 0.2.0-alpha.2 for jsonrpsee. Working on a fix now.

Signed-off-by: Gregory Hill <gregorydhill@outlook.com>
@gregdhill gregdhill force-pushed the subxt-client-unsubscribe branch from e92383d to 07e0fd5 Compare March 9, 2021 17:50
@niklasad1 niklasad1 merged commit c1d4804 into paritytech:master Mar 9, 2021
@gregdhill gregdhill deleted the subxt-client-unsubscribe branch March 9, 2021 22:01
@ascjones ascjones mentioned this pull request Mar 22, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants