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.

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

Removes cleanup command from run.sh after running rbac example #1226

Open
wants to merge 2 commits into
base: 7.5.x
Choose a base branch
from

Conversation

kc596
Copy link
Member

@kc596 kc596 commented Jul 11, 2023

Description

Removes cleanup after running rbac script run.sh as this voids above commands.

Author Validation

[X] security/rbac

Reviewer Tasks

Describe the tasks/validation that the PR submitter is requesting to be done by the reviewer.

Remove cleanup after run.
@kc596 kc596 requested a review from a team as a code owner July 11, 2023 10:36
@@ -44,4 +44,4 @@ sleep 1
./enable-rbac-ksqldb-server.sh
./enable-rbac-control-center.sh

./cleanup.sh
# ./cleanup.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

Looking back at history, run.sh was always intended to clean up after itself, but I don't see why it couldn't leave services running. That would also work better with the docs page that goes with the example since the step after Run example is Stop example

just delete the line rather than comment out though

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep I agree with @davetroiano

Copy link
Member Author

Choose a reason for hiding this comment

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

cleanup should be done separately for actually "running" and playing with the examples.

Deleted the line as suggested. Commented initially because it was like that in previous history.

Copy link
Contributor

@bbejeck bbejeck left a comment

Choose a reason for hiding this comment

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

This should go against the 7.4.0-post branch

@@ -44,4 +44,4 @@ sleep 1
./enable-rbac-ksqldb-server.sh
./enable-rbac-control-center.sh

./cleanup.sh
# ./cleanup.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

Yep I agree with @davetroiano

@kc596
Copy link
Member Author

kc596 commented Jul 12, 2023

This should go against the 7.4.0-post branch

Will raise separate PR for it. Because the branch history is different now.

@kc596 kc596 changed the title Update run.sh Removes cleanup command from run.sh after running rbac example Jul 12, 2023
Copy link

cla-assistant bot commented Apr 28, 2024

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

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.

None yet

3 participants