Skip to content

Add getWorkerTaskReachability API #1919

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 3 commits into from
Nov 3, 2023

Conversation

Quinn-With-Two-Ns
Copy link
Contributor

Add getWorkerTaskReachability API

closes #1862

@Quinn-With-Two-Ns Quinn-With-Two-Ns marked this pull request as ready for review November 1, 2023 15:04
@Quinn-With-Two-Ns Quinn-With-Two-Ns requested a review from a team as a code owner November 1, 2023 15:04
* the value is an empty list, the Build ID is not reachable on that queue.
*/
public Map<String, List<TaskReachability>> getTaskQueueReachability() {
return Collections.unmodifiableMap(taskQueueReachability);
Copy link
Member

Choose a reason for hiding this comment

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

I think this should be done in the constructor or by the constructor's caller (same with unmodifiable list below)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would prefer it to be here then in the constructor or trust the caller to have used the correct type

Copy link
Member

Choose a reason for hiding this comment

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

I see this is already merged, but curious why not the constructor? Why create a new object every time a getter is called if you don't need to?

Copy link
Member

@Sushisource Sushisource left a comment

Choose a reason for hiding this comment

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

Thanks for this! I hope this wasn't something I was supposed to go back and add and forgot about 😬

@Quinn-With-Two-Ns Quinn-With-Two-Ns merged commit 7077b54 into temporalio:master Nov 3, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support build ID reachability API
3 participants