Skip to content

ref(cells): silo client renames#110701

Open
lynnagara wants to merge 4 commits intomasterfrom
silo-client-renames
Open

ref(cells): silo client renames#110701
lynnagara wants to merge 4 commits intomasterfrom
silo-client-renames

Conversation

@lynnagara
Copy link
Member

No description provided.

@lynnagara lynnagara requested a review from a team March 13, 2026 22:48
@lynnagara lynnagara requested a review from a team as a code owner March 13, 2026 22:48
@lynnagara lynnagara changed the title ref(cell): silo client renames ref(cells): silo client renames Mar 13, 2026
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Mar 13, 2026
@github-actions
Copy link
Contributor

Backend Test Failures

Failures on 0f3a010 in this run:

tests/sentry/silo/test_client.py::SiloClientTest::test_invalid_cell_silo_ip_addresslog
tests/sentry/silo/test_client.py:331: in test_invalid_cell_silo_ip_address
    assert err.args == ("Disallowed Region Silo IP address: 172.31.255.255",)
E   AssertionError: assert ('Disallowed ....31.255.255',) == ('Disallowed ....31.255.255',)
E     
E     At index 0 diff: 'Disallowed Cell Silo IP address: 172.31.255.255' != 'Disallowed Region Silo IP address: 172.31.255.255'
E     
E     Full diff:
E       (
E     -     'Disallowed Region Silo IP address: 172.31.255.255',
E     ?                 ^ ^^^^
E     +     'Disallowed Cell Silo IP address: 172.31.255.255',
E     ?                 ^ ^^
E       )

Copy link
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

eu_region = Cell("eu", 1, eu_region_address, RegionCategory.MULTI_TENANT)
def test_get_cell_ip_addresses_when_single_host_invalid(mock_incr: MagicMock) -> None:
us1_cell_address = "http://i.am.us1.internal.hostname:9000"
us1_cell = Cell("eu", 1, us1_cell_address, RegionCategory.MULTI_TENANT)
Copy link
Contributor

Choose a reason for hiding this comment

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

Test cell names inconsistent with variable names

Low Severity

The rename changed variable names from eu_region/us_region to us1_cell/us2_cell and updated the hostnames accordingly (i.am.us1..., i.am.us2...), but the Cell constructor name arguments were left as "eu" and "us". So us1_cell is a Cell named "eu" with a us1 hostname, creating confusing test data that could mislead future developers.

Additional Locations (1)
Fix in Cursor Fix in Web

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

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant