Skip to content
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

Add support for debug traces in Check Bulk Permission #2193

Merged
merged 2 commits into from
Jan 8, 2025

Conversation

josephschorr
Copy link
Member

No description provided.

@josephschorr josephschorr requested a review from a team as a code owner January 7, 2025 21:44
@github-actions github-actions bot added area/CLI Affects the command line area/api v1 Affects the v1 API area/dependencies Affects dependencies area/tooling Affects the dev or user toolchain (e.g. tests, ci, build tools) area/dispatch Affects dispatching of requests labels Jan 7, 2025
@josephschorr josephschorr changed the title Add support for debug traces in Bulk Add support for debug traces in Check Bulk Permission Jan 7, 2025
@josephschorr josephschorr force-pushed the bulk-check-debug branch 2 times, most recently from ab1a858 to 4887d37 Compare January 8, 2025 00:02
@josephschorr josephschorr added this pull request to the merge queue Jan 8, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 8, 2025
@josephschorr josephschorr added this pull request to the merge queue Jan 8, 2025
Copy link
Contributor

@tstirrat15 tstirrat15 left a comment

Choose a reason for hiding this comment

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

LGTM, had a question

Copy link
Contributor

Choose a reason for hiding this comment

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

Curious - why did adding tracing to bulk check require adding the concept of a node ID? Is this because a bulk check can potentially break up its dispatch to different nodes and we want to identify them? Why wasn't it necessary in the unary check case?

Copy link
Member Author

Choose a reason for hiding this comment

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

It didn't; the node ID is an additional benefit for debugging

Merged via the queue into authzed:main with commit c18b165 Jan 8, 2025
23 checks passed
@josephschorr josephschorr deleted the bulk-check-debug branch January 8, 2025 17:02
@github-actions github-actions bot locked and limited conversation to collaborators Jan 8, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/api v1 Affects the v1 API area/CLI Affects the command line area/dependencies Affects dependencies area/dispatch Affects dispatching of requests area/tooling Affects the dev or user toolchain (e.g. tests, ci, build tools)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants