-
Notifications
You must be signed in to change notification settings - Fork 850
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
Adding ListQueues in History and Admin service #5110
Conversation
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.
This looks perfect except for the missing page token in the response. I think we need that for clients to paginate properly
} | ||
|
||
message ListQueuesResponse { | ||
repeated string queue_names = 1; |
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.
Won't clients need the next page token here?
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.
Yes. Added the token.
// enqueueing a task into the DLQ and then calling [getdlqtasks.Invoke] to verify that the right task is returned. | ||
func TestInvoke(t *testing.T, manager persistence.HistoryTaskQueueManager) { | ||
ctx := context.Background() | ||
t.Run("HappyPath", func(t *testing.T) { |
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.
Could we do a paginated query on the happy path? For example, add 3 queues, query through with a page size of 2 and verify that we get all 3 once the response's next page token is empty?
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.
Yeah added that to this test.
26b39b5
to
74b495c
Compare
What changed?
Adding ListQueues API to history service and admin service.
Why?
Operators will be using these APIs to get the list of DLQs.
A command will be added to tdbg to call this API.
How did you test it?
Unit tests with 100% coverage.
Potential risks
Is hotfix candidate?