-
Notifications
You must be signed in to change notification settings - Fork 932
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
Use jq instead of grep -P for finding users to delete during integrat… #2319
Conversation
…ion cleanup - was annoying me cause mac grep doesn't have -P
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!!
Cleaner than the previous implementation.
bin/cleanup-integration
Outdated
@@ -62,8 +62,7 @@ for stack in $(cf stacks | awk '/INTEGRATION-STACK/ { print $1 }'); do | |||
cf curl -X DELETE "/v3/stacks/$(cf stack --guid $stack)" | |||
done | |||
|
|||
CF_USERS=$(cf curl /v3/users | grep -oP '(?<="total_results":)[^"]*' |grep -o '[0-9]\+') | |||
USER_PAGES=$(( $CF_USERS / 50 + 1)) | |||
USER_PAGES=$(cf curl /v3/users | jq -r .pagination.total_pages) | |||
|
|||
for ((i=1; i<=${USER_PAGES}; i++)) ; do | |||
cf curl "/v3/users?per_page=50&page=${i}" | \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this line needs to remove the per_page=50
part because the change above I made is just getting the total number of paginated pages for whatever the default number of users of the page is
#2319) * Use jq instead of grep -P for finding users to delete during integration cleanup * remove per page cause we're already iterating over a known set of pages
#2319) * Use jq instead of grep -P for finding users to delete during integration cleanup * remove per page cause we're already iterating over a known set of pages
…ion cleanup
Thank you for contributing to the CF CLI! Please read the following:
If your contribution falls under a company CLA but your membership is not public, expect delays while we confirm.
Does this PR modify CLI v6, CLI v7, or CLI v8?
none it is testing related
Please see the contribution doc above or review Architecture Guide.
Description of the Change
We must be able to understand the design of your change from this description.
Keep in mind that the maintainer reviewing this PR may not be familiar with or
have worked with the code here recently, so please walk us through the concepts.
Why Is This PR Valuable?
What benefits will be realized by the code change? What users would want this change? What user need is this change addressing?
easier to understand clean up code
Why Should This Be In Core?
Explain why this functionality should be in the cf CLI, as opposed to a plugin.
it's integration test cleanup related
Applicable Issues
List any applicable Github Issues here
How Urgent Is The Change?
Is the change urgent? If so, explain why it is time-sensitive.
🤷
Other Relevant Parties
Who else is affected by the change?