Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Extended MockCluster to set a broker down and up again. Also added a … #992

Closed
wants to merge 3 commits into from
Closed

Extended MockCluster to set a broker down and up again. Also added a … #992

wants to merge 3 commits into from

Conversation

kkoehler
Copy link
Contributor

@kkoehler kkoehler commented May 4, 2023

Hi,

this PR adds the ability to set a MockCluster down and up again. With this change you're able to test not reachable clusters. A sample is also included.

Kristian

@milindl milindl self-requested a review May 5, 2023 05:26
milindl
milindl reviewed May 15, 2023
View reviewed changes
Copy link
Contributor

@milindl milindl 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 this PR! It mostly looks good to me, there are a few minor comments.

Besides those, please add the example to examples/README.md, and examples/.gitignore as well.

There probably needs to be a CHANGELOG.md entry as well, but I can add that later.

examples/mockcluster_failure_example/mockcluster.go Outdated Show resolved Hide resolved
examples/mockcluster_failure_example/mockcluster.go Outdated Show resolved Hide resolved
kafka/mockcluster.go Show resolved Hide resolved
@milindl milindl mentioned this pull request May 15, 2023
Merged
@milindl
Copy link
Contributor

milindl commented May 15, 2023

Looks good. Made an internal branch, and created a PR using that for CI to run before the merge. #998

@cla-assistant
Copy link

cla-assistant bot commented Aug 4, 2023 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@Sam-sad-Sajid
Copy link

Hi folks. I'm wondering if this feature will be released soon.

Sam-sad-Sajid
Sam-sad-Sajid reviewed Aug 8, 2023
View reviewed changes
examples/.gitignore
@@ -26,6 +26,7 @@ legacy/consumer_channel_example/consumer_channel_example
legacy/producer_channel_example/producer_channel_example
library-version/library-version
mockcluster_example/mockcluster_example
mockcluster_example/mockcluster_failure_example
Copy link

Choose a reason for hiding this comment

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

Just out of curiosity: why is this in .gitignore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

HI.
Right... both entries for the mock cluster examples seams to be unnecessary. Looks like a copy&paste error.

@milindl
Copy link
Contributor

milindl commented Oct 6, 2023

Hi @kkoehler , could you sign the CLA, we'll need that before merging
Thanks

@kkoehler
Copy link
Contributor Author

kkoehler commented Oct 6, 2023

Hi @kkoehler , could you sign the CLA, we'll need that before merging Thanks

sorry, i thought that i signed already. done

@milindl
Copy link
Contributor

milindl commented Oct 9, 2023

Right, it got reset, I think, for all contributors. Thanks for signing it again.

@milindl
Copy link
Contributor

milindl commented Oct 9, 2023 •
edited
Loading

Closing this as I merged the one on the internal branch with some minor changes and after running the CI. Thanks for this PR!

@milindl milindl closed this Oct 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Sam-sad-Sajid Sam-sad-Sajid Sam-sad-Sajid left review comments

@milindl milindl milindl left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kkoehler @milindl @Sam-sad-Sajid