Skip to content

fix(hc): Change RPC schemas to be represented as OpenAPI #68345

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

Closed
wants to merge 4 commits into from

Conversation

RyanSkonnord
Copy link
Contributor

Change RPC schemas that can't be represented in the OpenAPI format, generally because they rely on a fixed-length tuple with declared types at individual indexes. The success criterion is that the sentry rpcschema --diagnose command (not present in this branch, but can be merged locally from #68337) indicates 0 errors.

Because this branch covers breaking changes both to services and call sites, it will not be deployed to production as-is. We will use the five-step procedure for rolling out breaking RPC changes. However, to avoid having to redo steps, I'm hoping to get it reviewed all at once. (Most of the RPC changes are mechanistic, but there are a few design choices in the model changes -- names and such.)

Here are the RPC services being changed and associated errors from sentry rpcschema --diagnose being fixed:

  • OrganizationMappingService.upsert: value is not a valid dict (type=type_error.dict)
  • UserService.get_or_create_user_by_email: value is not a valid dict (type=type_error.dict)
  • ControlOrganizationProvisioningRpcService.bulk_create_organization_slug_reservations: value is not a valid dict (type=type_error.dict)
  • NotificationsService.get_subscriptions_for_projects: value could not be parsed to a boolean (type=type_error.bool)
  • IntegrationService.get_organization_context: value is not a valid dict (type=type_error.dict)
  • IntegrationService.get_organization_contexts: value is not a valid dict (type=type_error.dict)
  • ImportExportService.export_by_model: value could not be parsed to a boolean (type=type_error.bool)
  • ImportExportService.import_by_model: value could not be parsed to a boolean (type=type_error.bool)

I hope that's not too much to review at once. If it is, I could decompose it into separate PRs (but would then want to re-compose it for the sake of the five-step rollout process mentioned above, so yeah).

Remove an extra layer of model name keys from the `inserted_map`
attribute of a ControlImportChunk object inserted as setup. This has not
previously caused a test breakage because the path being tested does not
need to read the deserialized value, but it is inconsistent with the
type annotations on `PrimaryKeyMap.mapping` and will break when we add
validation when deserializing.
Copy link
Member

@markstory markstory left a comment

Choose a reason for hiding this comment

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

Looks good to me as a target state.

Copy link

codecov bot commented Apr 5, 2024

Bundle Report

Bundle size has no change ✅

@getsantry
Copy link
Contributor

getsantry bot commented Apr 27, 2024

This pull request has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you add the label WIP, I will leave it alone unless WIP is removed ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@getsantry getsantry bot added the Stale label Apr 27, 2024
@getsantry getsantry bot closed this May 5, 2024
markstory added a commit that referenced this pull request May 14, 2024
Mulligan on #68345 that resolves the import issue that required a
revert.
@github-actions github-actions bot locked and limited conversation to collaborators May 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants