Skip to content

Conversation

dill21yu
Copy link

What changes were proposed in this pull request?

Fix IndexError in HiveServer2 Address Parsing via ZooKeeper in #4146

Please review Hue Contributing Guide before opening a pull request.

Copy link

github-actions bot commented May 23, 2025

⚠️ No test files modified. Please ensure that changes are properly tested. ⚠️

Copy link

Python Code Coverage

Python Coverage Report •
FileStmtsMissCoverMissing
apps/beeswax/src/beeswax/server
   dbms.py78851934%107, 118, 123, 141–142, 168–169, 186–191, 200–201, 248–255, 257–260, 268–269, 283–284, 317, 319, 342, 346, 352, 359, 399, 405–406, 425–426, 428, 430–431, 433, 436, 439–441, 443–445, 447–448, 450–451, 453, 456, 458–459, 461, 463–464, 466, 468–470, 473, 475–476, 478, 480, 482–484, 487–490, 492–493, 495, 497–499, 502, 510, 513–517, 522–526, 528, 531–533, 535–536, 538, 540–546, 548–550, 552–553, 555–556, 558, 561–565, 568, 570–571, 573, 575–576, 578–579, 581–583, 585–586, 588–589, 591, 594, 597–598, 600–601, 604–605, 607–608, 611–612, 614–615, 618, 623–624, 626, 629, 632, 635, 637–640, 642–643, 645, 648–651, 654–655, 658–672, 674, 676, 678–684, 686, 688–690, 692–693, 695–697, 699, 728–729, 731, 733–735, 737, 739–744, 746, 748, 752–753, 755–759, 761, 764, 767–768, 770–772, 774, 776, 779, 781–782, 784–785, 787–790, 792–793, 795, 798–799, 801, 803–804, 806–809, 811–813, 815–816, 818, 831, 834, 844–846, 848–849, 851–854, 857, 859–860, 862–865, 868–871, 873, 882–883, 885–888, 890, 893–894, 896, 898, 907, 919–921, 923, 926, 928–930, 932–934, 936, 938–939, 941–942, 944, 947, 950, 952–954, 956–957, 959–960, 962, 965–967, 969, 972–977, 980–981, 984–988, 995–1000, 1005–1007, 1009, 1018–1019, 1021, 1023–1037, 1039, 1042–1043, 1046, 1049, 1052, 1068–1069, 1072–1080, 1082, 1085, 1087, 1090, 1092–1094, 1097–1099, 1101, 1107–1109, 1122, 1124, 1126–1132, 1134–1138, 1141–1148, 1150, 1156, 1159, 1165–1166, 1168, 1171–1172, 1174–1175, 1177–1178, 1180–1181, 1183, 1185–1186, 1188, 1191, 1194, 1196–1197, 1199–1200, 1202–1203, 1205–1206, 1208, 1211, 1213–1214, 1216–1218, 1220, 1223, 1229, 1231, 1233–1235, 1237, 1242–1243, 1245–1247, 1249, 1252, 1256–1257, 1259–1261, 1263, 1266, 1268–1269, 1271–1273, 1275, 1278, 1282, 1291, 1301, 1304, 1328, 1330–1341, 1346–1350, 1352–1353, 1355, 1357–1358, 1360–1361
TOTAL537342701349% 

Pytest Report

Tests Skipped Failures Errors Time
1090 106 💤 0 ❌ 0 🔥 5m 55s ⏱️

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes an IndexError in HiveServer2 address parsing by refining how ZooKeeper nodes are filtered and sorted based on sequence information. Key changes include initializing the hiveservers list instead of None, updating logging levels, and introducing a filtered and sorted list of nodes that contain a valid "sequence" pattern.

Comments suppressed due to low confidence (1)

apps/beeswax/src/beeswax/server/dbms.py:117

  • When no sequence nodes are found, the function returns the original hiveservers list, which may include nodes that do not match the expected sequence pattern. This could result in an IndexError downstream; consider explicitly handling this scenario or filtering out nodes that could cause issues.
else:

quadoss
quadoss previously approved these changes Jun 4, 2025
Copy link
Collaborator

@quadoss quadoss 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.

But there are test failures, Please review and address it.

apps/beeswax/src/beeswax/server/dbms.py:111:5: E265 [] Block comment should start with #
apps/beeswax/src/beeswax/server/dbms.py:118:48: W605 [
] Invalid escape sequence: \d

@dill21yu
Copy link
Author

dill21yu commented Jun 9, 2025

Please help review the code @wing2fly @amitsrivastava @ranade1

ranade1
ranade1 previously approved these changes Jun 9, 2025
Copy link
Contributor

@ranade1 ranade1 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.

Copy link

UI Coverage Report

Lines Statements Branches Functions
Coverage: 32%
39.15% (30527/77959) 31.01% (14247/45936) 23.89% (2130/8915)

Copy link

Python Code Coverage

Python Coverage Report •
FileStmtsMissCoverMissing
apps/beeswax/src/beeswax/server
   dbms.py78851934%107, 118, 123, 141–142, 168–169, 186–191, 200–201, 248–255, 257–260, 268–269, 283–284, 317, 319, 342, 346, 352, 359, 399, 405–406, 425–426, 428, 430–431, 433, 436, 439–441, 443–445, 447–448, 450–451, 453, 456, 458–459, 461, 463–464, 466, 468–470, 473, 475–476, 478, 480, 482–484, 487–490, 492–493, 495, 497–499, 502, 510, 513–517, 522–526, 528, 531–533, 535–536, 538, 540–546, 548–550, 552–553, 555–556, 558, 561–565, 568, 570–571, 573, 575–576, 578–579, 581–583, 585–586, 588–589, 591, 594, 597–598, 600–601, 604–605, 607–608, 611–612, 614–615, 618, 623–624, 626, 629, 632, 635, 637–640, 642–643, 645, 648–651, 654–655, 658–672, 674, 676, 678–684, 686, 688–690, 692–693, 695–697, 699, 728–729, 731, 733–735, 737, 739–744, 746, 748, 752–753, 755–759, 761, 764, 767–768, 770–772, 774, 776, 779, 781–782, 784–785, 787–790, 792–793, 795, 798–799, 801, 803–804, 806–809, 811–813, 815–816, 818, 831, 834, 844–846, 848–849, 851–854, 857, 859–860, 862–865, 868–871, 873, 882–883, 885–888, 890, 893–894, 896, 898, 907, 919–921, 923, 926, 928–930, 932–934, 936, 938–939, 941–942, 944, 947, 950, 952–954, 956–957, 959–960, 962, 965–967, 969, 972–977, 980–981, 984–988, 995–1000, 1005–1007, 1009, 1018–1019, 1021, 1023–1037, 1039, 1042–1043, 1046, 1049, 1052, 1068–1069, 1072–1080, 1082, 1085, 1087, 1090, 1092–1094, 1097–1099, 1101, 1107–1109, 1122, 1124, 1126–1132, 1134–1138, 1141–1148, 1150, 1156, 1159, 1165–1166, 1168, 1171–1172, 1174–1175, 1177–1178, 1180–1181, 1183, 1185–1186, 1188, 1191, 1194, 1196–1197, 1199–1200, 1202–1203, 1205–1206, 1208, 1211, 1213–1214, 1216–1218, 1220, 1223, 1229, 1231, 1233–1235, 1237, 1242–1243, 1245–1247, 1249, 1252, 1256–1257, 1259–1261, 1263, 1266, 1268–1269, 1271–1273, 1275, 1278, 1282, 1291, 1301, 1304, 1328, 1330–1341, 1346–1350, 1352–1353, 1355, 1357–1358, 1360–1361
TOTAL541852707850% 

Pytest Report

Tests Skipped Failures Errors Time
1186 106 💤 0 ❌ 0 🔥 6m 3s ⏱️

@dill21yu
Copy link
Author

dill21yu commented Jul 7, 2025

Hi @wing2fly @ramprasadagarwal @tabraiz12 @Harshg999 , the PR has passed all checks and reviews. Could you please approve the required workflows so it can be merged? Thanks!

Copy link

This PR is stale because it has been open 45 days with no activity and is not labeled "Prevent stale". Remove "stale" label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the Stale label Aug 23, 2025
@dill21yu dill21yu requested a review from ranade1 September 17, 2025 06:00
@dill21yu dill21yu dismissed stale reviews from quadoss and ranade1 via ce0823a September 22, 2025 09:59
@dill21yu dill21yu requested a review from quadoss September 23, 2025 07:07
@dill21yu
Copy link
Author

@quadoss @ranade1 I've fixed the code formatting issue that caused the CI failure. Could you please review when you have time? Also, could you trigger the CI pipeline? Thanks!

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.

5 participants