ref(cells): update region_name to cell_name in rpc dataclasses#111039
ref(cells): update region_name to cell_name in rpc dataclasses#111039
Conversation
this adds support for `cell_name` in RpcOrganizationMapping and RpcOrganizationMappingUpdate classes. `region_name` is still being sent on the wire currently. that will be flipped as the next step.
| if "cell_name" in values and "region_name" not in values: | ||
| values["region_name"] = values.pop("cell_name") | ||
| return values |
There was a problem hiding this comment.
Is this intended to be a forwards compatible shim for when the wire format changes to using cell_name?
There was a problem hiding this comment.
Yes. This is the first step so all we can flip all usages of .region_name to .cell_name first here and in getsentry and support both on the wire
2nd step will be to change the wire format
3rd step would be to remove the shim and fully rename the region_name property to cell_name
There was a problem hiding this comment.
That sounds like it will work.
Same idea as #111039 for the rest of the rpc models
| @root_validator(pre=True) | ||
| @classmethod | ||
| def _accept_cell_name(cls, values: dict[str, Any]) -> dict[str, Any]: | ||
| if "cell_name" in values and "region_name" not in values: |
There was a problem hiding this comment.
Should this also check that values['region_name'] is truthy to handle the removal process where a default value will be sent?
There was a problem hiding this comment.
I don't think so because:
- The default value
""isn't really valid here. An org mapping should always have a real cell/region - since this runs with
pre=True, this is the first step before any other validation or defaults are applied
the pydantic models RpcOrganizationMapping and RpcOrganizationMappingUpdate used for rpc now send `cell_name` instead of `region_name` on the wire support for both fields was added in #111039
this adds support for
cell_namein RpcOrganizationMapping and RpcOrganizationMappingUpdate classes.region_nameis still being sent on the wire currently. that will be flipped as the next step.