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

Azure deployment glue #118

Merged
merged 3 commits into from
Dec 7, 2018
Merged

Conversation

asyasky
Copy link
Collaborator

@asyasky asyasky commented Dec 5, 2018

Adds/updates the things we need to do an Azure Deployment using Azure DevOps Continuous Deployment. Sets up the framework for using Storage & KeyVault.

@asyasky asyasky requested a review from morganbr December 5, 2018 21:54
@@ -0,0 +1,33 @@
<?xml version="1.0" encoding="utf-8"?>
Copy link
Contributor

Choose a reason for hiding this comment

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

You seem to have two of these. Do we actually need both?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nope, good catch, thanks! :)

}
},
"ServerCore": {
"commandName": "Project",
"launchBrowser": true,
"environmentVariables": {
"ASPNETCORE_ENVIRONMENT": "Development"
"ASPNETCORE_ENVIRONMENT": "Development",
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you need to leave this on for now? If so, please file an issue to turn it off

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the only way to get useful error messages right now. I'm looking into other ways for the long term (supposedly I can turn on logging in the web.config, but the web.config is generated during Publish & I haven't traced that back to what I actually need to do). I'll add an issue.

@@ -14,6 +15,17 @@ public Startup(IConfiguration configuration)
Configuration = configuration;
}


Copy link
Contributor

Choose a reason for hiding this comment

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

Extra white space

services.AddDbContext<PuzzleServerContext>
(options => options.UseLazyLoadingProxies()
.UseSqlServer(Configuration.GetConnectionString("PuzzleServerContext")));
// Use SQL Database if in Azure, otherwise, use localdb
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like duplication of Program.cs. Which one do we actually need?

@@ -14,6 +15,17 @@ public Startup(IConfiguration configuration)
Configuration = configuration;
}


public Startup(IHostingEnvironment env)
Copy link
Contributor

Choose a reason for hiding this comment

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

Also looks like a dupe of Program.cs

}
else
{
app.UseDeveloperExceptionPage();
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this or file an issue to do so

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed (was for debugging)

});
}

// This method gets called by the runtime. Use this method to configure the HTTP request pipeline.
public void Configure(IApplicationBuilder app, IHostingEnvironment env)
{
if (env.IsDevelopment())
PuzzleServerContext.UpdateDatabase(app);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this how we actually want production db updates to happen?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That is an excellent question and I don't have an answer right now. This is the simplest way to take care of the database, but it does prevent us from having full control once we have real data.


services.AddDbContext<PuzzleServerContext>
(options => options.UseLazyLoadingProxies()
.UseSqlServer(context.Configuration.GetConnectionString("PuzzleServerSQLConnectionString")));
Copy link
Contributor

Choose a reason for hiding this comment

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

Please document where our secrets are and how to set them up as part of this so we can set up real production if we need it

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The secrets store isn't set up yet, right now we're using the Azure Application Settings - a future change will turn on KeyVault access and I'll add docs for using that.

@asyasky asyasky merged commit 9cfe259 into PuzzleServer:master Dec 7, 2018
@asyasky asyasky deleted the azureDeployRound1 branch December 7, 2018 04:48
asyasky added a commit to asyasky/mainpuzzleserver that referenced this pull request Jan 27, 2019
* Adds logic required to deploy to Azure using Continuous Deployment.
* Yaml updates
asyasky added a commit to asyasky/mainpuzzleserver that referenced this pull request Jan 31, 2019
* Adds logic required to deploy to Azure using Continuous Deployment.
* Yaml updates
asyasky added a commit to asyasky/mainpuzzleserver that referenced this pull request Feb 20, 2019
* Adds logic required to deploy to Azure using Continuous Deployment.
* Yaml updates
asyasky added a commit to asyasky/mainpuzzleserver that referenced this pull request Feb 26, 2019
* Adds logic required to deploy to Azure using Continuous Deployment.
* Yaml updates
asyasky added a commit to asyasky/mainpuzzleserver that referenced this pull request Feb 28, 2019
* Adds logic required to deploy to Azure using Continuous Deployment.
* Yaml updates
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.

2 participants