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

Custodian emits a new asset-receive-complete event to notification subscribers #659

Merged
merged 4 commits into from
Nov 27, 2023

Conversation

ffranr
Copy link
Contributor

@ffranr ffranr commented Nov 7, 2023

Fixes #650

@ffranr ffranr self-assigned this Nov 7, 2023
@ffranr ffranr requested review from jharveyb and Roasbeef November 7, 2023 18:28
@ffranr ffranr force-pushed the add-recv-complete-event branch from 03995f8 to fde04c9 Compare November 8, 2023 18:40
@dstadulis dstadulis changed the title Custodian emits a new asset recveive complete event to notification subscribers Custodian emits a new asset-receive-complete event to notification subscribers Nov 13, 2023
@ffranr ffranr force-pushed the add-recv-complete-event branch from fde04c9 to 0674091 Compare November 13, 2023 17:40
@ffranr ffranr requested a review from GeorgeTsagk November 13, 2023 17:41
@ffranr ffranr force-pushed the add-recv-complete-event branch 2 times, most recently from f45f63d to a36addd Compare November 13, 2023 18:31
Copy link
Member

@GeorgeTsagk GeorgeTsagk left a comment

Choose a reason for hiding this comment

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

Looks good! couple of minor comments

tapgarden/custodian.go Outdated Show resolved Hide resolved
itest/send_test.go Outdated Show resolved Hide resolved
@ffranr ffranr force-pushed the add-recv-complete-event branch from a36addd to 0fd34a9 Compare November 14, 2023 18:19
@dstadulis dstadulis added this to the v0.4 milestone Nov 16, 2023
@ffranr ffranr force-pushed the add-recv-complete-event branch 2 times, most recently from 3b75336 to 5437bc5 Compare November 21, 2023 18:59
Copy link
Contributor

@jharveyb jharveyb left a comment

Choose a reason for hiding this comment

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

Looks good, clean PR! 💯

Needs another rebase.

tapgarden/custodian.go Outdated Show resolved Hide resolved
AssetReceiveCompleteEvent is a new event added in this commit. It is
published by the custodian to status event subscribers once the asset
receive process is complete.
We were previously setting up an asynchronous routine and the
immediately blocking until the routine had finished. This commit
simplifies the code by removing that unnecessary asynchronous setup.
This commit adds an assertion function which blocks until the receiver
tapd node publishes an asset receive complete event.
@ffranr ffranr force-pushed the add-recv-complete-event branch from 5437bc5 to 6cacee9 Compare November 27, 2023 20:57
@ffranr ffranr added this pull request to the merge queue Nov 27, 2023
Merged via the queue into main with commit 346d838 Nov 27, 2023
14 checks passed
@guggero guggero deleted the add-recv-complete-event branch January 8, 2024 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

Custodian should emit an RPC notification status event when the recieve process completes sucessfully
4 participants