-
Notifications
You must be signed in to change notification settings - Fork 15
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
Only broadcast HostRemovedFromCluster
when a host is part of a cluster
#1611
Changes from 1 commit
d615d40
8f87ce1
6d4ac87
762e367
21834be
f406559
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -148,8 +148,12 @@ defmodule Trento.HostProjectorTest do | |
} | ||
|
||
ProjectorTestHelper.project(HostProjector, event, "host_projector") | ||
projection = Repo.get!(HostReadModel, host_id) | ||
|
||
assert_broadcast "host_details_updated", | ||
arbulu89 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
%{id: ^host_id}, | ||
1000 | ||
|
||
projection = Repo.get!(HostReadModel, host_id) | ||
assert nil == projection.cluster_id | ||
end | ||
|
||
|
@@ -169,8 +173,12 @@ defmodule Trento.HostProjectorTest do | |
} | ||
|
||
ProjectorTestHelper.project(HostProjector, event, "host_projector") | ||
projection = Repo.get!(HostReadModel, host_id) | ||
|
||
refute_broadcast "host_details_updated", | ||
%{id: ^host_id}, | ||
1000 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm just a bit worried about this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the 1000 value was set by me when I started doing broadcast test, this was set by testing manually different values, if the default timeout is sufficient we can switch to that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I mean, in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I reduced this timeout to 100 mS 👍 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a least request XD There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just did on my local machine and |
||
|
||
projection = Repo.get!(HostReadModel, host_id) | ||
assert cluster_id == projection.cluster_id | ||
end | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the correct payload to broadcast?
cluster_id
will always benil
because of the pattern-matching, is it useful to transmit this?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, we need to explicitily send
cluster_id: nil
, because this is the value we want to store in the frontend.If we don't send it, actually we are not sending any information.
This message is used to merge the already stored host data in redux with the new one, where we want to set the cluster as nil