-
Notifications
You must be signed in to change notification settings - Fork 134
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
Deploy App Manifest overrides #2924
Conversation
Hey nwmac! Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you and the commit authors have already signed the CLA. |
Codecov Report
@@ Coverage Diff @@
## v2-master #2924 +/- ##
=============================================
- Coverage 71.22% 71.19% -0.04%
=============================================
Files 606 607 +1
Lines 25982 26106 +124
Branches 5893 5910 +17
=============================================
+ Hits 18506 18585 +79
- Misses 7476 7521 +45 |
Still need to.. - Store overrides in app env var - Ensure redeploy flow works correctly (includes disabled state, showing original values, next button texts) - Show drop down for domains
- Add temp step size fix for create app's create route + register endpoint - Add domains list - Store overrides in stratos env var and load on redeploy
@@ -159,11 +168,20 @@ export class DeployApplicationDeployer { | |||
this.inputStream.next(JSON.stringify(msg)); | |||
} | |||
|
|||
sentAppOverride = (appOverrides: OverrideAppDetails) => { |
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.
send
sendEvent(clientWebSocket, OVERRIDES_REQUIRED) | ||
|
||
// Wait for a message from the client | ||
log.Info("Waiting for app overrides from client") |
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.
Debug
return err | ||
} | ||
|
||
log.Infof("Overrides: %v+", msgOverrides) |
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.
Debugf
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.
Check message type is OVERRIDES_SUPPLIED
@@ -100,22 +125,23 @@ func (cfAppPush *CFAppPush) deploy(echoContext echo.Context) error { | |||
return err | |||
} | |||
|
|||
log.Infof("%v+", msg) | |||
log.Infof("Source %v+", msg) |
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.
Debugf
|
||
var flags []string | ||
|
||
if len(overrides.Name) != 0 { |
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.
Could possible factor out to a helper function
This PR adds an extra (optional) step to the Deploy App flow to allow manifest overrides to be provided.