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

Print max-in-flight value alongside deployment status [main] #3098

Merged

Conversation

pivotalgeorge
Copy link
Contributor

@pivotalgeorge pivotalgeorge commented Aug 12, 2024

Description of the Change

This prints max-in-flight: <value> after the deployment status, when there is an Active deployment for an app and when max-in-flight is set to a non-default value

Why Is This PR Valuable?

It provides the user with information/confirmation that they have set a non-default max-in-flight value

Applicable Issues/PRs

this PR is based on #3096 and is therefore blocked on that PR getting merged into main

+@joaopapereira @weresch

@pivotalgeorge
Copy link
Contributor Author

rebasing on main after merge of #3096

@pivotalgeorge pivotalgeorge force-pushed the max-in-flight-status-main branch from a4a17a8 to ffa3f52 Compare August 12, 2024 20:13
Copy link
Contributor

@weresch weresch left a comment

Choose a reason for hiding this comment

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

issue (blocking): There was feedback on the v8 PR #3097 that applies here. Please include the additional commits from it.

Copy link
Contributor

@joaopapereira joaopapereira left a comment

Choose a reason for hiding this comment

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

LGTM

@pivotalgeorge
Copy link
Contributor Author

pivotalgeorge commented Aug 14, 2024

updated with additional commits from #3097

pivotalgeorge and others added 5 commits August 14, 2024 17:51
Co-authored-by: Greg Weresch <greg.weresch@broadcom.com>
…s table

when it is set to non-default value (currently 1)

Co-authored-by: Greg Weresch <greg.weresch@broadcom.com>
- these tests depend on a short-lived state

Co-authored-by: Greg Weresch <greg.weresch@broadcom.com>
Co-authored-by: João Pereira <joaod@vmware.com>
Co-authored-by: Greg Weresch <greg.weresch@broadcom.com>
@pivotalgeorge pivotalgeorge force-pushed the max-in-flight-status-main branch from 86a4e9a to a2bb971 Compare August 14, 2024 17:52
Copy link
Contributor

@joaopapereira joaopapereira left a comment

Choose a reason for hiding this comment

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

LGTM

@joaopapereira joaopapereira merged commit c904e40 into cloudfoundry:main Aug 14, 2024
18 checks passed
nicolasbender pushed a commit to sap-contributions/cli that referenced this pull request Nov 5, 2024
…undry#3098)

* Deduplicate tests from previous merge

Co-authored-by: Greg Weresch <greg.weresch@broadcom.com>

* Print max-in-flight value as part of deployment information in process table

when it is set to non-default value (currently 1)

Co-authored-by: Greg Weresch <greg.weresch@broadcom.com>

* Fix flakey tests that look for output from cf app

- these tests depend on a short-lived state

Co-authored-by: Greg Weresch <greg.weresch@broadcom.com>

* Apply suggestions from code review

Co-authored-by: João Pereira <joaod@vmware.com>

* Refactor app summary displayer tests to use Say instead of MatchRegexp

Co-authored-by: Greg Weresch <greg.weresch@broadcom.com>

---------

Co-authored-by: Greg Weresch <greg.weresch@broadcom.com>
Co-authored-by: João Pereira <joaod@vmware.com>
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.

4 participants