Push and scale app and run-task #2303
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
|
/easycla |
7739f18 to
4c24ae0
Compare
…cloudfoundry#2278) Bumps [rack](https://github.com/rack/rack) from 2.2.3 to 2.2.3.1. - [Release notes](https://github.com/rack/rack/releases) - [Changelog](https://github.com/rack/rack/blob/main/CHANGELOG.md) - [Commits](rack/rack@2.2.3...2.2.3.1) --- updated-dependencies: - dependency-name: rack dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
4c24ae0 to
1d623f6
Compare
* since `cf push` uses manifests this commit also adds support for log rate limit in manifests Signed-off-by: Carson Long <lcarson@vmware.com> Signed-off-by: Rebecca Roberts <robertsre@vmware.com>
* app summary displayer can now handle rendering log rate and log rate limit metrics for running instances * app summaries will show `0 of 0` if the cloud controller does not support log rate limit container metrics Signed-off-by: Rebecca Roberts <robertsre@vmware.com> Signed-off-by: Matthew Kocher <mkocher@vmware.com> Signed-off-by: Rebecca Roberts <robertsre@vmware.com> Signed-off-by: Matthew Kocher <mkocher@vmware.com> Signed-off-by: Rebecca Roberts <robertsre@vmware.com>
* update help text to suggest using ='s with -1 to avoid flag parsing problem Signed-off-by: Ben Fuller <benjaminf@vmware.com> Signed-off-by: Matthew Kocher <mkocher@vmware.com>
* this affects non-manifest based commands Signed-off-by: Matthew Kocher <mkocher@vmware.com>
1d623f6 to
25e36f9
Compare
jdgonzaleza
reviewed
Aug 22, 2022
| @@ -197,7 +249,7 @@ var _ = Describe("app summary displayer", func() { | |||
| It("lists instance stats for process types that have > 0 instances", func() { | |||
| Expect(testUI.Out).To(Say(`type:\s+web`)) | |||
| Expect(testUI.Out).To(Say(`sidecars: `)) | |||
| Expect(testUI.Out).To(Say(`state\s+since\s+cpu\s+memory\s+disk\s+details`)) | |||
| Expect(testUI.Out).To(Say(`state\s+since\s+cpu\s+memory\s+disk\s+logging\s+details`)) | |||
There was a problem hiding this comment.
(Very) Nitpicky: Perhaps we can have a constant with the "title column" state\s+since\s+cpu\s+memory\s+disk\s+logging\s+details ?
I'm not against of the current implementation. It's just an idea.
jdgonzaleza
reviewed
Aug 22, 2022
| @@ -3,7 +3,7 @@ GEM | |||
| specs: | |||
| mustermann (1.1.1) | |||
| ruby2_keywords (~> 0.0.1) | |||
| rack (2.2.3) | |||
| rack (2.2.3.1) | |||
There was a problem hiding this comment.
Hmmm, I think this came from rebasing our changes against main, I'm thinking it'll drop out when we rebase before merging it into main.
jdgonzaleza
reviewed
Aug 22, 2022
| @@ -58,8 +59,8 @@ func ParseV3AppProcessTable(input []byte) AppTable { | |||
| // instance row | |||
| columns := splitColumns(row) | |||
| details := "" | |||
| if len(columns) >= 7 { | |||
| details = columns[6] | |||
| if len(columns) >= 8 { | |||
There was a problem hiding this comment.
Is it 8 the number of columns we have? if that's the case, could we create a constant for that instead of hardcoding it ?
jdgonzaleza
reviewed
Aug 22, 2022
| @@ -105,7 +105,7 @@ var _ = Describe("restart command", func() { | |||
| Eventually(session).Should(Say(`type:\s+web`)) | |||
| Eventually(session).Should(Say(`instances:\s+1/1`)) | |||
| Eventually(session).Should(Say(`memory usage:\s+\d+(M|G)`)) | |||
| Eventually(session).Should(Say(`\s+state\s+since\s+cpu\s+memory\s+disk\s+details`)) | |||
| Eventually(session).Should(Say(`\s+state\s+since\s+cpu\s+memory\s+disk\s+logging\s+details`)) | |||
There was a problem hiding this comment.
Same (very) nitpicky suggestion as above. 😄
pivotalgeorge
approved these changes
Aug 27, 2022
4 tasks
jdgonzaleza
pushed a commit
that referenced
this pull request
Sep 7, 2022
* Bump rack from 2.2.3 to 2.2.3.1 in /fixtures/applications/example-app (#2278) * `cf push` command accepts log rate limit flag * since `cf push` uses manifests this commit also adds support for log rate limit in manifests * `cf scale` command accepts log rate limit flag * app summary displayer can now handle rendering log rate and log rate limit metrics for running instances * app summaries will show `0 of 0` if the cloud controller does not support log rate limit container metrics * `cf run-task` command accepts log rate limit flag * update help text to suggest using ='s with -1 to avoid flag parsing problem * Flag parser accepts 0B, -1B, 0T, etc for flags * this affects non-manifest based commands * Extract constants for instance stats columns * Extract constant for column count Signed-off-by: Carson Long <lcarson@vmware.com> Signed-off-by: Rebecca Roberts <robertsre@vmware.com> Signed-off-by: Ben Fuller <benjaminf@vmware.com> Signed-off-by: Matthew Kocher <mkocher@vmware.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
pushaccepts --log-rate-limit / -l flag.scaleaccepts --log-rate-limit / -l flag.run-taskaccepts --log-rate-limit / -l flag.