-
Notifications
You must be signed in to change notification settings - Fork 160
feat: Updates to deployment process to enable rollback (Part 1) #183
Conversation
| }; | ||
| } | ||
|
|
||
| private async runKuduCommand(functionApp: Site, command: string) { |
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.
This is not being used anywhere
PIC123
left a comment
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.
Looks good! Adding the options to the login service is what is going to help with both caching the login credentials and specifying region
070d2bb to
1278669
Compare
| deploy: | ||
| # Rollback enabled, deploying to blob storage | ||
| # Default is true | ||
| # If false, deploys directly to function app |
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.
nice, i like the flexibility 👍
| logStreamApiPath: "/api/logstream/application/functions/function/", | ||
| masterKeyApiPath: "/api/functions/admin/masterkey", | ||
| providerName: "azure", | ||
| rollbackEnabled: false, |
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.
the README says the default is true -- whichever we choose, just need to be consistent.
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.
Right, the default in this PR is false just to not break everything. The next PR will set this to true
|
|
||
| private async getToken(): Promise<string> { | ||
| const authResponse = await AzureLoginService.login({ | ||
| tokenAudience: "https://storage.azure.com/" |
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.
if this (https://storage.azure.com/) is only used here, I'm good, otherwise let's promote to a const somewhere.
pjlittle
left a comment
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.
Just a couple comments. Looks good - great foundation for rollback work.
Updating the deployment process will take a few pull requests to keep them (relatively) small. Here are the changes included in this pull request.
- Update default naming convention of Azure Storage account (and a utility function that assists with creating name)
- Stubs for rollback enabled (currently switched off to avoid breaking tests, will switch on in future PR), which includes
- Timestamping names of deployments
- Timestamping names of packages
- Initialization of deployment artifact container
- Updates the `LoginService` to accept options, which is used in the `AzureBlobStorageService` to retrieve a storage-specific access token
- Remove dead code from `FunctionAppService`
Resolves [AB#357], [AB#408], [AB#414], [AB#415]
Updating the deployment process will take a few pull requests to keep them (relatively) small. Here are the changes included in this pull request.
LoginServiceto accept options, which is used in theAzureBlobStorageServiceto retrieve a storage-specific access tokenFunctionAppServiceResolves [AB#357], [AB#408], [AB#414], [AB#415]