Skip to content

Conversation

@naspirato
Copy link
Collaborator

@naspirato naspirato commented Sep 9, 2025

Changelog entry

This PR refactors the URL length limiting logic to use binary search for finding the maximum error text length that fits within GitHub's URL constraints. The change replaces pre-calculated space estimations with actual URL length testing to more accurately determine how much error text can be included.

  • Replaces space estimation with precise URL length checking

Changelog category

  • Not for changelog (changelog entry is not required)

Description for reviewers

...

@github-actions
Copy link

github-actions bot commented Sep 9, 2025

🟢 2025-09-15 20:15:01 UTC The validation of the Pull Request description is successful.

@github-actions
Copy link

github-actions bot commented Sep 9, 2025

2025-09-09 20:57:48 UTC Pre-commit check linux-x86_64-relwithdebinfo for 14103ba has started.
2025-09-09 20:58:08 UTC Artifacts will be uploaded here
2025-09-09 21:01:07 UTC ya make is running...
🟢 2025-09-09 21:01:13 UTC Tests successful.

Test history | Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
0 0 0 0 0 0

🟢 2025-09-09 21:01:20 UTC Build successful.

@github-actions
Copy link

github-actions bot commented Sep 9, 2025

2025-09-09 20:57:48 UTC Pre-commit check linux-x86_64-release-asan for 14103ba has started.
2025-09-09 20:58:25 UTC Artifacts will be uploaded here
2025-09-09 21:01:35 UTC ya make is running...
🟢 2025-09-09 21:01:41 UTC Tests successful.

Test history | Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
0 0 0 0 0 0

🟢 2025-09-09 21:01:47 UTC Build successful.

@naspirato naspirato requested a review from Copilot September 9, 2025 20:58

This comment was marked as outdated.

@naspirato naspirato self-assigned this Sep 10, 2025
@github-actions
Copy link

github-actions bot commented Sep 10, 2025

2025-09-10 05:20:18 UTC Pre-commit check linux-x86_64-relwithdebinfo for a2d98d3 has started.
2025-09-10 05:20:31 UTC Artifacts will be uploaded here
2025-09-10 05:23:33 UTC ya make is running...
2025-09-10 05:25:42 UTC Check cancelled

@github-actions
Copy link

github-actions bot commented Sep 10, 2025

2025-09-10 05:20:18 UTC Pre-commit check linux-x86_64-release-asan for a2d98d3 has started.
2025-09-10 05:20:50 UTC Artifacts will be uploaded here
2025-09-10 05:23:39 UTC ya make is running...
2025-09-10 05:25:43 UTC Check cancelled

@github-actions
Copy link

github-actions bot commented Sep 10, 2025

2025-09-10 05:26:18 UTC Pre-commit check linux-x86_64-release-asan for 4716d74 has started.
2025-09-10 05:26:25 UTC Artifacts will be uploaded here
2025-09-10 05:28:56 UTC ya make is running...
2025-09-10 05:31:05 UTC Check cancelled

@github-actions
Copy link

github-actions bot commented Sep 10, 2025

2025-09-10 05:26:18 UTC Pre-commit check linux-x86_64-relwithdebinfo for 4716d74 has started.
2025-09-10 05:26:23 UTC Artifacts will be uploaded here
2025-09-10 05:28:59 UTC ya make is running...
2025-09-10 05:31:05 UTC Check cancelled

@github-actions
Copy link

github-actions bot commented Sep 10, 2025

2025-09-10 05:31:40 UTC Pre-commit check linux-x86_64-release-asan for 7902213 has started.
2025-09-10 05:31:45 UTC Artifacts will be uploaded here
2025-09-10 05:34:13 UTC ya make is running...

@github-actions
Copy link

github-actions bot commented Sep 10, 2025

2025-09-10 05:31:40 UTC Pre-commit check linux-x86_64-relwithdebinfo for 7902213 has started.
2025-09-10 05:31:45 UTC Artifacts will be uploaded here
2025-09-10 05:34:24 UTC ya make is running...

@naspirato naspirato requested a review from Copilot September 10, 2025 05:35
Copy link
Contributor

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 refactors the GitHub issue URL generation logic to use binary search for finding the maximum error text length that fits within GitHub's URL constraints, replacing the previous estimation-based approach with precise URL length testing.

  • Implements binary search algorithm with iteration limit to find optimal error text truncation
  • Replaces space estimation constants and logic with actual URL length checking
  • Simplifies log section handling by removing space-based filtering constraints

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
ydb/tests/functional/tpc/medium/test_workload_manager.py Adds a comment line (cosmetic change)
.github/scripts/tests/templates/summary.html Refactors URL length limiting from estimation-based to binary search approach with precise URL testing

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@naspirato naspirato added ok-to-test Special label used to approve a PR for testing on our infrastructure rebase-and-check Rebase PR with the current base branch and check labels Sep 10, 2025
@naspirato naspirato changed the title max url lenght now will be find with binaty search HTML report: max url lenght now will be find with binaty search Sep 10, 2025
@naspirato naspirato added rebase-and-check Rebase PR with the current base branch and check and removed rebase-and-check Rebase PR with the current base branch and check labels Sep 10, 2025
@github-actions github-actions bot removed the ok-to-test Special label used to approve a PR for testing on our infrastructure label Sep 10, 2025
@github-actions
Copy link

github-actions bot commented Sep 10, 2025

2025-09-10 08:20:40 UTC Pre-commit check linux-x86_64-relwithdebinfo for 1659737 has started.
2025-09-10 08:20:47 UTC Artifacts will be uploaded here
2025-09-10 08:23:18 UTC ya make is running...
🟢 2025-09-10 08:23:24 UTC Tests successful.

Test history | Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
0 0 0 0 0 0

🟢 2025-09-10 08:23:30 UTC Build successful.

@naspirato naspirato enabled auto-merge (squash) September 10, 2025 08:20
@github-actions
Copy link

github-actions bot commented Sep 10, 2025

2025-09-10 08:20:46 UTC Pre-commit check linux-x86_64-release-asan for 1659737 has started.
2025-09-10 08:20:55 UTC Artifacts will be uploaded here
2025-09-10 08:23:28 UTC ya make is running...
🟢 2025-09-10 08:23:34 UTC Tests successful.

Test history | Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
0 0 0 0 0 0

🟢 2025-09-10 08:23:40 UTC Build successful.

@github-actions
Copy link

github-actions bot commented Sep 10, 2025

2025-09-10 09:08:34 UTC Pre-commit check linux-x86_64-release-asan for 7ec10ad has started.
2025-09-10 09:09:09 UTC Artifacts will be uploaded here
2025-09-10 09:12:18 UTC ya make is running...
🟢 2025-09-10 09:12:24 UTC Tests successful.

Test history | Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
0 0 0 0 0 0

🟢 2025-09-10 09:12:31 UTC Build successful.

@github-actions
Copy link

github-actions bot commented Sep 10, 2025

2025-09-10 09:21:37 UTC Pre-commit check linux-x86_64-relwithdebinfo for 7ec10ad has started.
2025-09-10 09:22:17 UTC Artifacts will be uploaded here
2025-09-10 09:25:33 UTC ya make is running...
🟢 2025-09-10 09:25:39 UTC Tests successful.

Test history | Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
0 0 0 0 0 0

2025-09-10 09:25:47 UTC Check cancelled

@naspirato naspirato force-pushed the binary_search_for_max_lenght_of_url branch from 4206714 to 64c201f Compare September 10, 2025 09:25
@github-actions
Copy link

github-actions bot commented Sep 10, 2025

2025-09-10 09:41:49 UTC Pre-commit check linux-x86_64-relwithdebinfo for f01674e has started.
2025-09-10 09:41:53 UTC Artifacts will be uploaded here
2025-09-10 09:44:28 UTC ya make is running...
🟢 2025-09-10 09:44:34 UTC Tests successful.

Test history | Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
0 0 0 0 0 0

🟢 2025-09-10 09:44:42 UTC Build successful.

@github-actions
Copy link

github-actions bot commented Sep 10, 2025

2025-09-10 10:26:21 UTC Pre-commit check linux-x86_64-release-asan for f01674e has started.
2025-09-10 10:26:25 UTC Artifacts will be uploaded here
2025-09-10 10:28:48 UTC ya make is running...
🟢 2025-09-10 10:28:54 UTC Tests successful.

Test history | Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
0 0 0 0 0 0

🟢 2025-09-10 10:29:01 UTC Build successful.

@naspirato naspirato changed the title HTML report: max url lenght now will be find with binaty search HTML report: max url lenght now will be find after ecoding Sep 15, 2025
@github-actions
Copy link

github-actions bot commented Sep 15, 2025

2025-09-15 20:14:58 UTC Pre-commit check linux-x86_64-release-asan for 71900f7 has started.
2025-09-15 20:15:39 UTC Artifacts will be uploaded here
2025-09-15 20:18:32 UTC ya make is running...
🟢 2025-09-15 20:18:38 UTC Tests successful.

Test history | Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
0 0 0 0 0 0

🟢 2025-09-15 20:18:44 UTC Build successful.

@naspirato naspirato requested a review from Copilot September 15, 2025 20:16
@github-actions
Copy link

github-actions bot commented Sep 15, 2025

2025-09-15 20:16:57 UTC Pre-commit check linux-x86_64-relwithdebinfo for 71900f7 has started.
2025-09-15 20:17:36 UTC Artifacts will be uploaded here
2025-09-15 20:19:56 UTC Check cancelled

Copy link
Contributor

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

Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

function showCopiedTooltip() {}

// Safe percent-encoded truncation helpers
function isDecodable(s) {
Copy link

Copilot AI Sep 15, 2025

Choose a reason for hiding this comment

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

This function should handle the case where the input parameter is null or undefined to prevent potential runtime errors.

Suggested change
function isDecodable(s) {
function isDecodable(s) {
if (typeof s !== 'string') return false;

Copilot uses AI. Check for mistakes.
Comment on lines +514 to +516
while (cut && !isDecodable(cut)) {
if (/%[0-9A-Fa-f]{2}$/.test(cut)) cut = cut.slice(0, -3);
else cut = cut.slice(0, -1);
Copy link

Copilot AI Sep 15, 2025

Choose a reason for hiding this comment

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

This while loop could potentially run for a long time with malformed input. Consider adding a maximum iteration limit to prevent infinite loops.

Suggested change
while (cut && !isDecodable(cut)) {
if (/%[0-9A-Fa-f]{2}$/.test(cut)) cut = cut.slice(0, -3);
else cut = cut.slice(0, -1);
let maxIterations = 1000, iter = 0;
while (cut && !isDecodable(cut) && iter < maxIterations) {
if (/%[0-9A-Fa-f]{2}$/.test(cut)) cut = cut.slice(0, -3);
else cut = cut.slice(0, -1);
iter++;

Copilot uses AI. Check for mistakes.
@github-actions
Copy link

github-actions bot commented Sep 15, 2025

2025-09-15 20:23:44 UTC Pre-commit check linux-x86_64-relwithdebinfo for 2ebc13a has started.
2025-09-15 20:23:59 UTC Artifacts will be uploaded here
2025-09-15 20:26:27 UTC ya make is running...
🟢 2025-09-15 20:28:26 UTC Tests successful.

Test history | Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
41 41 0 0 0 0

🟢 2025-09-15 20:28:33 UTC Build successful.

@github-actions
Copy link

github-actions bot commented Sep 15, 2025

2025-09-15 20:23:51 UTC Pre-commit check linux-x86_64-release-asan for 2ebc13a has started.
2025-09-15 20:24:04 UTC Artifacts will be uploaded here
2025-09-15 20:26:30 UTC ya make is running...
🟡 2025-09-15 20:31:40 UTC Some tests failed, follow the links below. This fail is not in blocking policy yet

Test history | Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
39 9 0 30 0 0

🟢 2025-09-15 20:31:47 UTC Build successful.

@github-actions
Copy link

github-actions bot commented Sep 15, 2025

2025-09-15 22:02:44 UTC Pre-commit check linux-x86_64-relwithdebinfo for 0ae1328 has started.
2025-09-15 22:03:05 UTC Artifacts will be uploaded here
2025-09-15 22:06:05 UTC ya make is running...
🟢 2025-09-15 22:06:10 UTC Tests successful.

Test history | Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
0 0 0 0 0 0

🟢 2025-09-15 22:06:16 UTC Build successful.

@github-actions
Copy link

github-actions bot commented Sep 15, 2025

2025-09-15 22:03:32 UTC Pre-commit check linux-x86_64-release-asan for 0ae1328 has started.
2025-09-15 22:03:46 UTC Artifacts will be uploaded here
2025-09-15 22:06:21 UTC ya make is running...
🟢 2025-09-15 22:06:26 UTC Tests successful.

Test history | Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
0 0 0 0 0 0

🟢 2025-09-15 22:06:33 UTC Build successful.

@naspirato naspirato merged commit f1595ce into ydb-platform:main Sep 16, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants