fix(test_user_batch_custom_time): handle stress command correctly #9345
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fruch
force-pushed
the
followup_on_4684
branch
from
856cab9
to
048911f
Compare
fruch
force-pushed
the
followup_on_4684
branch
from
048911f
to
34c1090
Compare
roydahan
previously approved these changes
Nov 25, 2024
vponomaryov
previously requested changes
Nov 25, 2024
seems like we were running into situations we try to add all nodes ips into stress commands, and it's the test code ``` File ".../longevity_test.py", line 300, in test_user_batch_custom_time self._run_user_stress_in_batches(batch_size=batch_size, File ".../longevity_test.py", line 336, in _run_user_stress_in_batches batch_params['stress_cmd'] += [params['stress_cmd'] + nodes_ips] TypeError: list indices must be integers or slices, not str ``` in this change remove a parentheses that was causing function to return tuple and not string as expected
fruch
force-pushed
the
followup_on_4684
branch
from
34c1090
to
a508927
Compare
fruch
commented
Nov 25, 2024
| # for now we'll leave to just one fresh table, to kick schema update | ||
| num_of_newly_created_tables = 1 | ||
| self._pre_create_templated_user_schema(batch_start=extra_tables_idx, | ||
| batch_end=extra_tables_idx+num_of_newly_created_tables) | ||
| for i in range(num_of_newly_created_tables): | ||
| batch.append(self.create_templated_user_stress_params(extra_tables_idx + i, cs_profile=cs_profile)) |
There was a problem hiding this comment.
this is the core of the fix
|
the test in 2024.2 reproducers, did proved this is fixing the issue. |
soyacz
approved these changes
Nov 26, 2024
vponomaryov
reviewed
Nov 26, 2024
| @@ -109,6 +109,8 @@ pipeline { | |||
| steps { | |||
| script { | |||
| try { | |||
| checkoutQaInternal(params) | |||
There was a problem hiding this comment.
Why is it needed here?
There was a problem hiding this comment.
cause the unittest is going via code path that depends on configuration files existing in the internal repo
i.e. the 5000 tables case has the user profile yaml there.
This was referenced Nov 26, 2024
scylladbbot
added
backport/6.2-done
and removed
backport/6.2
backport/2024.2
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
seems like we were running into situations we try to add all nodes ips into stress commands, and it's the test code
in this change remove a parentheses that was causing function to return tuple and not string as expected
Testing
PR pre-checks (self review)
backportlabelsReminders
sdcm/sct_config.py)unit-test/folder)