Jump to conversation
Unresolved conversations (3)
@matthiashanel matthiashanel Nov 22, 2020
I'd say the above check finds the loop here? how about A export a, B import a to b and export b, C import b to c to d and export d Then another test that rewrites subjects via mappings as well.
test/service_latency_test.go
derekcollison matthiashanel
@matthiashanel matthiashanel Nov 22, 2020
Are we doing direct cycles here? I would have expected a depth first search alternating between exports/imports facilitating a queue to avoid recursion.
server/accounts.go
matthiashanel derekcollison
@matthiashanel matthiashanel Nov 22, 2020
looks like this would trigger if: a importFormsCycle b a serviceExportOverlaps with c b serviceImportOverlaps with d Seems the checks do not take accounts fully into account?
server/accounts.go
matthiashanel derekcollison
Resolved conversations (6)
@matthiashanel matthiashanel Nov 22, 2020
This covers export -> mapping -> import ?
server/accounts.go
matthiashanel derekcollison
@matthiashanel matthiashanel Nov 21, 2020
Seems like a test with 3 accounts is warranted as well.
test/service_latency_test.go
@matthiashanel matthiashanel Nov 21, 2020
Not an actual code change but maybe add a todo in all the places in this and the previous PR so that in a subsequent change we can set a flag in either the client or the account to maybe record this event? Similar to incomplete in the account. If a cycle is the only way this can happen then maybe the account. if this can happen for some other reason (say reconnect) maybe in the client.
Outdated
server/client.go
derekcollison matthiashanel
@kozlovic kozlovic Nov 21, 2020
Is tabulation here intentional?
test/service_latency_test.go
derekcollison
@kozlovic kozlovic Nov 21, 2020
Is tabulation here intentional?
test/service_latency_test.go
derekcollison
@matthiashanel matthiashanel Nov 21, 2020
Would this also happen if the cycle would be bigger, A - B -C - A?
Outdated
server/accounts.go
derekcollison