-
Notifications
You must be signed in to change notification settings - Fork 58
[2/n] move instance_migrate and APIs used by tests/omdb from nexus-internal to nexus-lockstep #9037
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
Conversation
55291f4
to
a68433d
Compare
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.
Thank you for grinding through this! I love how many packages no longer have a dependency on nexus-client
. I just had one note here.
mock.assert_calls_async(1).await; | ||
|
||
// Deleting a subscription that doesn't exist should 404. | ||
dbg!( |
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.
I'm surprised this got deleted instead of converted.
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.
I meant to write a note here. This test was calling an endpoint that does not exist on the internal APIs, only the external ones (DELETE /v1/alert-receivers/{receiver}/subscriptions/{subscription}
). Because it does not exist, the API returns 404, and the test here "succeeds".
But now that I look at this again, I should be able to swap it for ctx.external_client
and it should be testing the correct thing.
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.
Replacing it for the external client doesn't work as the delete endpoint returns 204 No Content even after the subscription was deleted.
a68433d
to
6255457
Compare
6255457
to
8e3d4f5
Compare
Part of #8902. Stacked on #8983.