Skip to content

Tests: Add utility class TuningConfigBuilder to make IndexTask tests more readable and concise#16732

Merged
kfaraz merged 4 commits intoapache:masterfrom
kfaraz:test_tuning_config
Jul 15, 2024
Merged

Tests: Add utility class TuningConfigBuilder to make IndexTask tests more readable and concise#16732
kfaraz merged 4 commits intoapache:masterfrom
kfaraz:test_tuning_config

Conversation

@kfaraz
Copy link
Contributor

@kfaraz kfaraz commented Jul 13, 2024

Changes

  • No functional change
  • Add class TuningConfigBuilder to build IndexTuningConfig, CompactionTuningConfig
  • Remove old class ParallelIndexTestingFactory.TuningConfigBuilder
  • Remove some unused fields and methods

@kfaraz kfaraz requested a review from AmatyaAvadhanula July 13, 2024 13:36
@github-actions github-actions bot added Area - Batch Ingestion Area - Ingestion Area - MSQ For multi stage queries - https://github.com/apache/druid/issues/12262 labels Jul 13, 2024
@kfaraz kfaraz changed the title Tests: Add TuningConfigBuilder to make IndexTask tests more readable and concise Tests: Add utility class TuningConfigBuilder to make IndexTask tests more readable and concise Jul 13, 2024
@kfaraz kfaraz requested a review from abhishekrb19 July 13, 2024 14:29
.forCompactionTask()
.withMaxRowsInMemory(500000)
.withMaxBytesInMemory(1000000L)
.withMaxTotalRows(Long.MAX_VALUE)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, might have been copied over from somewhere else.
Will get rid of it in a follow up if that's okay (to avoid re-triggering CI.)

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure!

Copy link
Contributor

@AmatyaAvadhanula AmatyaAvadhanula left a comment

Choose a reason for hiding this comment

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

Thanks for the changes, @kfaraz!

@kfaraz
Copy link
Contributor Author

kfaraz commented Jul 15, 2024

Thanks a lot for the quick review, @AmatyaAvadhanula !

@kfaraz kfaraz merged commit 656667e into apache:master Jul 15, 2024
@kfaraz kfaraz deleted the test_tuning_config branch July 15, 2024 04:43
sreemanamala pushed a commit to sreemanamala/druid that referenced this pull request Aug 6, 2024
…more readable and concise (apache#16732)

Changes:
- No functional change
- Add class `TuningConfigBuilder` to build `IndexTuningConfig`, `CompactionTuningConfig`
- Remove old class `ParallelIndexTestingFactory.TuningConfigBuilder`
- Remove some unused fields and methods
@kfaraz kfaraz added this to the 31.0.0 milestone Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area - Batch Ingestion Area - Ingestion Area - MSQ For multi stage queries - https://github.com/apache/druid/issues/12262 Refactoring

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants