Skip to content

KAFKA-3728 EndToEndAuthorizationTest offsets_topic misconfigured#1425

Closed
edoardocomar wants to merge 1 commit intoapache:trunkfrom
edoardocomar:KAFKA-3728
Closed

KAFKA-3728 EndToEndAuthorizationTest offsets_topic misconfigured#1425
edoardocomar wants to merge 1 commit intoapache:trunkfrom
edoardocomar:KAFKA-3728

Conversation

@edoardocomar
Copy link
Copy Markdown
Contributor

@edoardocomar edoardocomar commented May 24, 2016

Set OffsetsTopicReplicationFactorProp to 3 like MinInSyncReplicasProp Else a consumer was able to consume via assign but not via subscribe, so the testProduceAndConsume is now duplicated to check both paths

@edoardocomar
Copy link
Copy Markdown
Contributor Author

Hi @ijuma I've implemented your suggestions.

I got rid of the TopicAuthorizationException on __consumer_offsets but
I could not get rid of OffsetCommitRequiredAcksProp=1

otherwise testProduceConsumeViaSubscribe fails if it's not the first one to be
executed (the test passes on its own).

Disappointing, this may be a cleanup problem but I could not spot any.

@ijuma
Copy link
Copy Markdown
Member

ijuma commented Jun 1, 2016

@edoardocomar, sorry for the delay. I investigated the issue and I think #1455 should fix it (I briefly tested your PR with it).

@edoardocomar
Copy link
Copy Markdown
Contributor Author

edoardocomar commented Jun 1, 2016

thanks @ijuma does this mean that I should be able to get rid of
OffsetCommitRequiredAcksProp=1 ?

@ijuma
Copy link
Copy Markdown
Member

ijuma commented Jun 1, 2016

Yes.

asfgit pushed a commit that referenced this pull request Jun 3, 2016
I found both issues while investigating the issue described in PR #1425.

Author: Ismael Juma <ismael@juma.me.uk>

Reviewers: Sriharsha Chintalapani <schintalapani@hortonworks.com>, Jun Rao <junrao@gmail.com>

Closes #1455 from ijuma/fix-integration-test-harness-and-zk-test-harness

(cherry picked from commit 1029030)
Signed-off-by: Ismael Juma <ismael@juma.me.uk>
asfgit pushed a commit that referenced this pull request Jun 3, 2016
I found both issues while investigating the issue described in PR #1425.

Author: Ismael Juma <ismael@juma.me.uk>

Reviewers: Sriharsha Chintalapani <schintalapani@hortonworks.com>, Jun Rao <junrao@gmail.com>

Closes #1455 from ijuma/fix-integration-test-harness-and-zk-test-harness
@ijuma
Copy link
Copy Markdown
Member

ijuma commented Jun 3, 2016

@edoardocomar, can you please rebase this now that #1455 was merged?

Copy link
Copy Markdown
Member

@ijuma ijuma Jun 3, 2016

Choose a reason for hiding this comment

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

Maybe call this setAclsAndProduce()

gfodor pushed a commit to AltspaceVR/kafka that referenced this pull request Jun 3, 2016
I found both issues while investigating the issue described in PR apache#1425.

Author: Ismael Juma <ismael@juma.me.uk>

Reviewers: Sriharsha Chintalapani <schintalapani@hortonworks.com>, Jun Rao <junrao@gmail.com>

Closes apache#1455 from ijuma/fix-integration-test-harness-and-zk-test-harness
Set OffsetsTopicReplicationFactorProp to 3 like MinInSyncReplicasProp

Else a consumer was able to consume via assign but not via subscribe,
so the testProduceAndConsume is now duplicated to check both paths
@edoardocomar
Copy link
Copy Markdown
Contributor Author

Thanks @ijuma with your changes in #1455 the only fix needed for this defect was to
Set OffsetsTopicReplicationFactorProp to 3 like MinInSyncReplicasProp
without which a consumer could not consume vai subscribe.

The other changes are no longer needed

@ijuma
Copy link
Copy Markdown
Member

ijuma commented Jun 3, 2016

LGTM @edoardocomar. Can you please update the PR description to match the commit message? The PR message is what gets used by our PR merge script in the merge commit.

I'll merge it to trunk and 0.10.0 once you do that and the tests pass. Merging to 0.10.0 to make it easier to backport KAFKA-3396.

@edoardocomar edoardocomar changed the title KAFKA-3728 EndToEndAuthorizationTest offsets_topic misconfigured KAFKA-3728 EndToEndAuthorizationTest offsets_topic misconfigured Set OffsetsTopicReplicationFactorProp to 3 like MinInSyncReplicasProp Else a consumer was able to consume via assign but not via subscribe, so the testProduceAndConsume is now duplicated to Jun 3, 2016
@edoardocomar edoardocomar changed the title KAFKA-3728 EndToEndAuthorizationTest offsets_topic misconfigured Set OffsetsTopicReplicationFactorProp to 3 like MinInSyncReplicasProp Else a consumer was able to consume via assign but not via subscribe, so the testProduceAndConsume is now duplicated to KAFKA-3728 EndToEndAuthorizationTest offsets_topic misconfigured Jun 3, 2016
@edoardocomar
Copy link
Copy Markdown
Contributor Author

Hi @ijuma all the tests pass for me - unlike in the Jenkins build. How do I re-kick a Jenkins build?
thanks

@ijuma
Copy link
Copy Markdown
Member

ijuma commented Jun 3, 2016

I'll run the tests locally. Can you please not repeat the PR title in the description?

@edoardocomar
Copy link
Copy Markdown
Contributor Author

done thx

@asfgit asfgit closed this in 49ddc89 Jun 3, 2016
asfgit pushed a commit that referenced this pull request Jun 3, 2016
Set OffsetsTopicReplicationFactorProp to 3 like MinInSyncReplicasProp  Else a consumer was able to consume via assign but not via subscribe, so the testProduceAndConsume is now duplicated to check both paths

Author: Edoardo Comar <ecomar@uk.ibm.com>

Reviewers: Ismael Juma <ismael@juma.me.uk>

Closes #1425 from edoardocomar/KAFKA-3728

(cherry picked from commit 49ddc89)
Signed-off-by: Ismael Juma <ismael@juma.me.uk>
@edoardocomar edoardocomar deleted the KAFKA-3728 branch June 6, 2016 11:58
granthenke pushed a commit to granthenke/kafka that referenced this pull request Oct 24, 2016
I found both issues while investigating the issue described in PR apache#1425.

Author: Ismael Juma <ismael@juma.me.uk>

Reviewers: Sriharsha Chintalapani <schintalapani@hortonworks.com>, Jun Rao <junrao@gmail.com>

Closes apache#1455 from ijuma/fix-integration-test-harness-and-zk-test-harness

(cherry picked from commit 1029030)
Signed-off-by: Ismael Juma <ismael@juma.me.uk>
granthenke pushed a commit to granthenke/kafka that referenced this pull request Oct 24, 2016
Set OffsetsTopicReplicationFactorProp to 3 like MinInSyncReplicasProp  Else a consumer was able to consume via assign but not via subscribe, so the testProduceAndConsume is now duplicated to check both paths

Author: Edoardo Comar <ecomar@uk.ibm.com>

Reviewers: Ismael Juma <ismael@juma.me.uk>

Closes apache#1425 from edoardocomar/KAFKA-3728

(cherry picked from commit 49ddc89)
Signed-off-by: Ismael Juma <ismael@juma.me.uk>
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.

2 participants