Skip to content

feat(ziti-management): add token counting service type#137

Closed
casey-brooks wants to merge 1 commit into
mainfrom
noa/issue-136
Closed

feat(ziti-management): add token counting service type#137
casey-brooks wants to merge 1 commit into
mainfrom
noa/issue-136

Conversation

@casey-brooks
Copy link
Copy Markdown
Contributor

Summary

  • add SERVICE_TYPE_TOKEN_COUNTING to the ziti-management ServiceType enum

Testing

  • buf lint
  • buf breaking --against '.git#branch=main'

Refs #136

@casey-brooks casey-brooks requested a review from a team as a code owner April 25, 2026 01:35
@casey-brooks
Copy link
Copy Markdown
Contributor Author

Test & Lint Summary

  • buf lint (lint: no errors)
  • buf breaking --against '.git#branch=main' (breaking: no errors)

@casey-brooks
Copy link
Copy Markdown
Contributor Author

Test & Lint Summary

  • buf lint (lint: no errors)
  • buf breaking --against '.git#branch=main' (tests: passed: 1, failed: 0, skipped: 0)

Copy link
Copy Markdown

@noa-lucent noa-lucent left a comment

Choose a reason for hiding this comment

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

Requesting changes.

Key blockers:

  • PR scope mismatch: #136 asks only for SERVICE_TYPE_TOKEN_COUNTING, but this PR introduces a large set of additional API surface changes across multiple services. Please split into focused PRs (with linked issues/specs) or update the PR description/scope so intent can be reviewed coherently.
  • Runner log streaming API changes appear semantically breaking (container_name documented as required + shifting end/error to gRPC status codes). Please add an explicit backwards-compat/versioning plan so existing clients are not broken.
  • Avoid duplicated schema shapes with “keep fields in sync” comments (e.g., WorkloadContainer vs runners.v1.Container). Extract a shared proto/type.

Once the above is addressed, happy to re-review.

Comment thread proto/agynio/api/ziti_management/v1/ziti_management.proto
Comment thread proto/agynio/api/apps/v1/apps.proto
Comment thread proto/agynio/api/apps/v1/apps.proto
Comment thread proto/agynio/api/runner/v1/runner.proto
Comment thread proto/agynio/api/runner/v1/runner.proto
Comment thread proto/agynio/api/threads/v1/threads.proto
Comment thread proto/agynio/api/notifications/v1/notifications.proto
@github-actions
Copy link
Copy Markdown

The latest Buf updates on your PR. Results from workflow buf-pr / buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedApr 25, 2026, 1:48 AM

@casey-brooks
Copy link
Copy Markdown
Contributor Author

Summary

  • reset the branch to main to drop unrelated proto edits
  • added SERVICE_TYPE_TOKEN_COUNTING to the ziti-management ServiceType enum

Test & Lint Summary

  • buf lint (lint: no errors)
  • buf breaking --against '.git#branch=main' (breaking: no errors)

@casey-brooks
Copy link
Copy Markdown
Contributor Author

Test & Lint Summary

  • buf lint (lint: no errors)
  • buf breaking --against '.git#branch=main' (passed: 1, failed: 0, skipped: 0)

Copy link
Copy Markdown

@noa-lucent noa-lucent left a comment

Choose a reason for hiding this comment

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

Re-review complete: PR is now properly scoped to #136 and adds only SERVICE_TYPE_TOKEN_COUNTING = 7 to ziti_management.v1.ServiceType.

No further changes requested.

@rowan-stein
Copy link
Copy Markdown
Collaborator

Superseded by updated architecture: token counting is now embedded inside agn (no standalone service / no Ziti enrollment needed), so adding SERVICE_TYPE_TOKEN_COUNTING is no longer required. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants