feat(billing): Add protobuf definitions for the seats service - #414
Open
volokluev wants to merge 4 commits into
Open
feat(billing): Add protobuf definitions for the seats service#414volokluev wants to merge 4 commits into
volokluev wants to merge 4 commits into
Conversation
Define proto messages for the billing platform seats service endpoints: - SeatObject: shared seat entity message - CheckAssignSeats: request/response with SeatOutcome enum - AssignSeat: request/response - DisableSeat, RemoveSeat, UpdateSeatIdentifier: request messages - GetSeatStatusForExternalProduct: request/response with ExternalProductSeatStatus enum - GetDailySeatUsageByProject: response with ProjectSeatUsage - RolloverSeatsToNewContract: request message
|
The latest Buf updates on your PR. Results from workflow ci / buf-checks (pull_request).
|
Member
Author
|
Companion getsentry PR: https://github.com/getsentry/getsentry/pull/21710 |
Comment on lines
+11
to
+14
| } | ||
|
|
||
| message GetDailySeatUsageByProjectResponse { | ||
| repeated ProjectSeatUsage projects = 1; |
There was a problem hiding this comment.
Bug: The endpoint_get_daily_seat_usage_by_project.proto file defines a response message but is missing the corresponding request message, making the endpoint unusable.
Severity: HIGH
Suggested Fix
Define a GetDailySeatUsageByProjectRequest message within endpoint_get_daily_seat_usage_by_project.proto. This message should include necessary parameters for the query, similar to other request messages in the service, such as organization_id and date range fields.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location:
proto/sentry_protos/billing/v1/services/seats/v1/endpoint_get_daily_seat_usage_by_project.proto#L11-L14
Potential issue: The protobuf definition for the `GetDailySeatUsageByProject` endpoint
in `endpoint_get_daily_seat_usage_by_project.proto` includes a
`GetDailySeatUsageByProjectResponse` message but omits the corresponding
`GetDailySeatUsageByProjectRequest` message. Other query endpoints in the billing
service, such as `GetUsageByProject`, define both request and response messages. Without
a defined request message, clients have no contract for how to call this new endpoint,
rendering it unusable.
Did we get this right? 👍 / 👎 to inform future reviews.
Move SeatObject from services/seats/v1/ to billing/v1/ since it is a shared type used across multiple service boundaries. Update all service protos to import from the new location.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
proto/sentry_protos/billing/v1/services/seats/v1/SeatObject, request/response messages for all seat service endpoints, and enums (SeatOutcome,ExternalProductSeatStatus)Proto files
seat_object.proto— sharedSeatObjectmessageendpoint_check_assign_seats.proto—CheckAssignSeatsRequest/Response,SeatOutcomeenumendpoint_assign_seat.proto—AssignSeatRequest/Responseendpoint_disable_seat.proto—DisableSeatRequestendpoint_remove_seat.proto—RemoveSeatRequestendpoint_update_seat_identifier.proto—UpdateSeatIdentifierRequestendpoint_get_seat_status.proto—GetSeatStatusForExternalProductRequest/Response,ExternalProductSeatStatusenumendpoint_get_daily_seat_usage_by_project.proto—GetDailySeatUsageByProjectResponse,ProjectSeatUsageendpoint_rollover_seats.proto—RolloverSeatsToNewContractRequestCompanion PR
Test plan
make build-py