Skip to content
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

Fix hostname printing in error messages #3343

Merged
merged 2 commits into from
Oct 11, 2023

Conversation

ptoscano
Copy link
Contributor

@ptoscano ptoscano commented Oct 10, 2023

  • change ConnectionOSErrorException.host to be a normalized hostname
  • normalize the hostname in a specific error message that does not use ExceptionMapper

These changes will fix the printing of IPv6 addresses in error messages.

Card ID: CCT-9

@cnsnyder cnsnyder requested review from a team and jirihnidek and removed request for a team October 10, 2023 14:09
@github-actions
Copy link

github-actions bot commented Oct 10, 2023

Coverage

Coverage (computed on Fedora latest) •
FileStmtsMissCoverMissing
rhsm
   connection.py101045554%48–49, 53, 55–56, 81, 101–102, 150, 284, 315, 381–386, 390–399, 460, 462, 564, 567, 574–580, 585, 641, 676–680, 682, 695, 722, 725–726, 728–729, 731, 742–746, 750, 754, 756–757, 784, 787, 791–792, 797, 800–801, 816, 820, 822–823, 850–856, 858, 860–867, 869, 872, 877, 880, 882, 884–888, 890–891, 893–898, 900–901, 903–904, 911–920, 922, 925–931, 933–934, 936–937, 948–953, 965–968, 973, 1037, 1039–1044, 1046, 1051–1055, 1061–1064, 1066–1071, 1075–1080, 1087, 1124, 1126, 1131, 1142, 1151–1154, 1158, 1160–1162, 1166–1167, 1169–1176, 1178, 1180, 1183–1190, 1193–1194, 1199, 1201, 1251, 1268–1271, 1295, 1317, 1347, 1352, 1355, 1358–1359, 1364, 1367, 1372, 1375, 1418–1422, 1429–1430, 1432, 1440–1441, 1443, 1460, 1473–1475, 1478, 1491, 1498, 1502, 1530–1532, 1537–1538, 1540–1541, 1543–1544, 1546–1560, 1562–1564, 1566–1577, 1579, 1596–1598, 1600–1602, 1604–1606, 1611, 1616–1618, 1623, 1650, 1682–1710, 1715–1716, 1718–1720, 1723–1724, 1727–1728, 1731–1732, 1751–1752, 1761–1762, 1772–1773, 1780–1781, 1787–1790, 1796–1799, 1805–1806, 1812–1813, 1833–1834, 1843–1847, 1855–1856, 1882–1885, 1910–1911, 1920–1921, 1929–1930, 1951, 1953–1955, 1957, 1959, 1962, 1964–1977, 1979–1980, 1989–1991, 2003–2004, 2013–2014, 2016, 2018–2020, 2027–2029, 2038–2040, 2048–2049, 2060, 2062–2063, 2065, 2067–2070, 2072–2074, 2077, 2079, 2086–2087, 2094–2095, 2105–2106, 2116–2119, 2129–2135, 2142–2145
subscription_manager/cli_command
   cli.py2093185%65, 69, 124, 128–129, 144, 201, 205, 207, 250, 289, 300–302, 316–318, 340, 360, 386, 395–396, 400–401, 417, 419–420, 422–423, 428, 436
TOTAL18157456174% 

Tests Skipped Failures Errors Time
2636 9 💤 0 ❌ 0 🔥 55.445s ⏱️

@m-horky
Copy link
Contributor

m-horky commented Oct 11, 2023

I can still see the incorrectly formatted address:

$ ((4e71f02ba...)) subscription-manager register --serverurl=[fe80::5]:1234/prefix
Unable to reach the server at fe80::5:1234/prefix

See cli.py:389.

Use the existing helper to provide a properly formatted hostname for
user displaying (e.g. for exception); this will fix the printing of
IPv6 addresses.

Turn the existing "host" member as property, so the current users keep
working as expected.
Normalize the hostname printed for connection errors when pinging the
server, in case anything was passed via --serverurl, --insecure, or
--baseurl.
@ptoscano
Copy link
Contributor Author

I can still see the incorrectly formatted address:

$ ((4e71f02ba...)) subscription-manager register --serverurl=[fe80::5]:1234/prefix
Unable to reach the server at fe80::5:1234/prefix

See cli.py:389.

Ah, the joy of custom exception handling... fixed this one easily.

@ptoscano ptoscano changed the title connection: normalize hostname in ConnectionOSErrorException Fix hostname printing in error messages Oct 11, 2023
@ptoscano ptoscano force-pushed the ptoscano/fix-hostname-print-in-exception branch from 4e71f02 to d045ae2 Compare October 11, 2023 13:25
Copy link
Contributor

@m-horky m-horky left a comment

Choose a reason for hiding this comment

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

LGTM

@m-horky m-horky merged commit 1587f09 into main Oct 11, 2023
10 checks passed
@m-horky m-horky deleted the ptoscano/fix-hostname-print-in-exception branch October 11, 2023 14:08
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.

2 participants