Skip to content

MINOR: KStreams SuppressScenarioTest should set StreamsConfig.STATE_DIR_CONFIG.#5826

Merged
ijuma merged 1 commit intoapache:trunkfrom
lbradstreet:suppress-test-statedir-config
Oct 27, 2018
Merged

MINOR: KStreams SuppressScenarioTest should set StreamsConfig.STATE_DIR_CONFIG.#5826
ijuma merged 1 commit intoapache:trunkfrom
lbradstreet:suppress-test-statedir-config

Conversation

@lbradstreet
Copy link
Copy Markdown
Contributor

@lbradstreet lbradstreet commented Oct 22, 2018

This sets StreamsConfig.STATED_DIR_CONFIG in KStreams SuppressScenarioTest, as with StreamsTestUtils. I have deliberately avoided using StreamsTestUtils as this test sets bogus config parameters, but still fails if the default STATE_DIR_CONFIG does not exist.

@lbradstreet
MINOR: SuppressScenarioTest should set StreamsConfig.STATE_DIR_CONFIG
aa2fd64 
to temp directory as in other KStreams tests.
@cmccabe
Copy link
Copy Markdown
Contributor

cmccabe commented Oct 22, 2018

Failed test was kafka.server.DynamicBrokerReconfigurationTest.testUncleanLeaderElectionEnable, whih looks unrelated.

@cmccabe
Copy link
Copy Markdown
Contributor

cmccabe commented Oct 22, 2018

Retest this please

@cmccabe
Copy link
Copy Markdown
Contributor

cmccabe commented Oct 23, 2018

LGTM. +1

@lbradstreet
Copy link
Copy Markdown
Contributor Author

lbradstreet commented Oct 23, 2018 •
edited
Loading

JDK 11 / Scala 2.12 failures look unrelated:

14:36:41 kafka.api.CustomQuotaCallbackTest > testCustomQuotaCallback FAILED
14:36:41     java.lang.AssertionError: Partition [group1_largeTopic,69] metadata not propagated after 15000 ms

14:40:45 kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsNotExistingGroup FAILED
14:40:45     java.util.concurrent.ExecutionException: org.apache.kafka.common.errors.CoordinatorNotAvailableException: The coordinator is not available.

@cmccabe
Copy link
Copy Markdown
Contributor

cmccabe commented Oct 23, 2018

retest this please

@vvcephei
Copy link
Copy Markdown
Contributor

vvcephei commented Oct 23, 2018

Hey @lbradstreet ,

Thanks for the PR. Can you elaborate on why this is necessary?

Thanks,
-John

@lbradstreet
Copy link
Copy Markdown
Contributor Author

lbradstreet commented Oct 24, 2018

Hi @vvcephei, all Kafka Client/Streams integration tests are required to use the TestUtils for temp directories rather than rely on StreamsConfig.STATED_DIR_CONFIG defaults.

@lbradstreet
Copy link
Copy Markdown
Contributor Author

lbradstreet commented Oct 24, 2018

Retest this please

@vvcephei
Copy link
Copy Markdown
Contributor

vvcephei commented Oct 24, 2018

Ah, I see that that's the case. Sorry for overlooking it, @lbradstreet. I'm also +1.

@ijuma
Copy link
Copy Markdown
Member

ijuma commented Oct 26, 2018

retest this please

1 similar comment
@lbradstreet
Copy link
Copy Markdown
Contributor Author

lbradstreet commented Oct 26, 2018

retest this please

@ijuma
Copy link
Copy Markdown
Member

ijuma commented Oct 27, 2018

JDK 8 build passed and JDK 11 build failed with known flaky test testMarksPartitionsAsOfflineAndPopulatesUncleanableMetrics (there's a separate PR for fixing it). Merging to trunk.

@ijuma ijuma merged commit c9d3deb into apache:trunk Oct 27, 2018
@lbradstreet lbradstreet mentioned this pull request Oct 27, 2018
Merged
ijuma pushed a commit that referenced this pull request Oct 28, 2018
@lbradstreet @ijuma
MINOR: SuppressionIntegrationTest should set StreamsConfig.STATE_DIR_…
7cdc433 
…CONFIG (#5847)

Sets StreamsConfig.STATED_DIR_CONFIG to temp directory in
SuppressionIntegrationTest, to match StreamsTestUtils.

This is a similar fix to #5826.

Reviewers: Ismael Juma <ismael@juma.me.uk>
@lbradstreet lbradstreet mentioned this pull request Nov 1, 2018
Merged
pengxiaolong pushed a commit to pengxiaolong/kafka that referenced this pull request Jun 14, 2019
@lbradstreet @pengxiaolong
MINOR: SuppressScenarioTest should set StreamsConfig.STATE_DIR_CONFIG (…
54e7c5a 
…apache#5826)

Set `StreamsConfig.STATED_DIR_CONFIG` in `SuppressScenarioTest`, as
with `StreamsTestUtils`. I have deliberately avoided using `StreamsTestUtils` as
this test sets bogus config parameters, but still fails if the default
`STATE_DIR_CONFIG` does not exist.

Reviewers: Colin Patrick McCabe <colin@cmccabe.xyz>, John Roesler <john@confluent.io>, Ismael Juma <ismael@juma.me.uk>
pengxiaolong pushed a commit to pengxiaolong/kafka that referenced this pull request Jun 14, 2019
@lbradstreet @pengxiaolong
MINOR: SuppressionIntegrationTest should set StreamsConfig.STATE_DIR_…
417f2e3 
…CONFIG (apache#5847)

Sets StreamsConfig.STATED_DIR_CONFIG to temp directory in
SuppressionIntegrationTest, to match StreamsTestUtils.

This is a similar fix to apache#5826.

Reviewers: Ismael Juma <ismael@juma.me.uk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@lbradstreet @cmccabe @vvcephei @ijuma