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

Flex Deployment #3404

Merged
merged 16 commits into from
Aug 9, 2023
Merged

Flex Deployment #3404

merged 16 commits into from
Aug 9, 2023

Conversation

khkh-ms
Copy link
Contributor

@khkh-ms khkh-ms commented Jul 17, 2023

Issue describing the changes in this PR

resolves #issue_for_this_pr

Pull request checklist

  • My changes do not require documentation changes
    • Otherwise: Documentation issue linked to PR
  • My changes do not need to be backported to a previous version
    • Otherwise: Backport tracked by issue/PR #issue_or_pr
  • I have added all required tests (Unit tests, E2E tests)

@khkh-ms khkh-ms requested a review from amamounelsayed as a code owner July 31, 2023 01:34
@@ -7,7 +7,7 @@ namespace Azure.Functions.Cli.Arm
internal static class ArmUriTemplates
{
public const string ArmApiVersion = "2018-09-01";
public const string WebsitesApiVersion = "2015-08-01";
public const string WebsitesApiVersion = "2015-08-01";//"2022-03-01-privatepreview&stamp=eliasl-linux01geo.eastus.cloudapp.azure.com";//"2015-08-01";
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we still need the comment?

}
else if (status == DeployStatus.Failed)
{
throw new CliException("The deployment failed, Please check the printed logs.");
Copy link
Contributor

Choose a reason for hiding this comment

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

We should print the deployment logs since the kudu instance will get recycled and there might be no way for customers to access the logs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At this point, logs are already printed. Please search "// Safely printing logs after the status is confirmed." in the code.

}
else if (status == DeployStatus.PartialSuccess)
{
ColoredConsole.WriteLine(WarningColor(GetLogMessage("\"Deployment was partially successful, Please check the printed logs.")));
Copy link
Contributor

Choose a reason for hiding this comment

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

Same thing as above. We should print the deployment logs since the kudu instance will get recycled and there might be no way for customers to access the logs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Logs should already be printed at this stage.

Copy link
Contributor

@kamperiadis kamperiadis left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you

@khkh-ms khkh-ms merged commit 76aca0e into v4.x Aug 9, 2023
@khkh-ms khkh-ms deleted the khkh/cv2-deployment-beta branch August 9, 2023 15:38
@khkh-ms khkh-ms restored the khkh/cv2-deployment-beta branch August 9, 2023 21:17
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.

3 participants