-
Notifications
You must be signed in to change notification settings - Fork 68
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
Make App Deployable without Server #65
Conversation
…ervices-web-calling-tutorial into without-server
@@ -8,27 +8,57 @@ products: | |||
- azure-communication-services | |||
--- | |||
|
|||
[![Deploy To Azure](https://raw.githubusercontent.com/Azure/azure-quickstart-templates/master/1-CONTRIBUTION-GUIDE/images/deploytoazure.svg?sanitize=true)](https://portal.azure.com/#create/Microsoft.Template/uri/https%3A%2F%2Fraw.githubusercontent.com%2FAzure-Samples%2Fcommunication-services-web-calling-tutorial%2Fmain%2Fdeploy%2Fazuredeploy.json) |
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.
What are these links? I dont have access to these links. Are these specific to your azure account?
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.
I see, i think this is for deplyment i think your mentioning it in the "Deplyment to azure... " section.
Why dont you just put the buttons there? Seems confusing to have them up here.
Also, what happens when i click on those buttons? Does it open up somethin in my browser and i have to do more things? Or will it just go ahead and deploy automatically? Please explain exactly wherw/how to get the deplyment url, will it show me in vscode console?
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 new folder your adding, "deploy" with the .bicep and .json, please explain in the readme what these are doing
}, | ||
"scripts": { | ||
"start": "webpack-dev-server --port 5000 --mode development", | ||
"build": "webpack --mode development" | ||
"build": "npm run clean && webpack --mode=production --env production", |
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.
You will need to be able to also run this locally(Make sure you test locally and remote). I would expect to also see scripts here for building locally.
Also, make sure when building locally it can still refresh local app when making local changes in the app code
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 scripts that were there before for building locally were:
"start": "webpack-dev-server --port 5000 --mode development",
"build": "webpack --mode development"
maybe you can name these "start-local" and "build-local" or as needed.
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.
Usually "build" is used for dev mode buildling. Id leave build for dev and do build-prod for prod/deplyment
@@ -0,0 +1,16 @@ | |||
<configuration> |
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.
Is this file needed? If so, explain what it is/doing in the readme
@@ -0,0 +1,41 @@ | |||
name: Create Sample Release |
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.
Same here please explain what this is doing in the readme.
Is this something that happens locally or during deplyment?
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.
Explain about the github token
} | ||
|
||
const communicationIdentityClient = new CommunicationIdentityClient(config.connectionString); | ||
const env = process.env; | ||
env.development = 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.
You cannot hard code this, it needs to work both locally and deplyment
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 variable basically gets set when running webpack and passing the env flag. If you pass env prod, this flag will be set to true. It is not something that you should set on your own.
|
||
const PORT = process.env.port || 8080; | ||
const isProd = env.development == null || |
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.
You should not have to do so many checks to see if your in prod. One flag check should be enough to truly kno wether ur in prod or not.
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.
For example, a script can just passe flag "--env production", this tells your your going to build in prod and end up building in prod
|
||
module.exports = { | ||
devtool: 'inline-source-map', | ||
mode: 'development', | ||
...(isProd ? {} : { devtool: 'eval-source-map' }), |
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.
why eval instead of inline. What is the difference?
} | ||
|
||
const communicationIdentityClient = new CommunicationIdentityClient(config.connectionString); | ||
const env = process.env; | ||
env.development = 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.
This variable basically gets set when running webpack and passing the env flag. If you pass env prod, this flag will be set to true. It is not something that you should set on your own.
|
||
## Deployment to Azure | ||
|
||
Use the blue buttons above. |
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.
So this will bring me to a portal where i have to do more things. You need to explain what i have to do in the portal.
Also, i should be able to deploy this manually therough the azure app service extension in vscode:
I right click on an app service and then click on "Deply to web app". We have always been deploying this way so this mehtod of deplyment should also work. It will deploy to .azurewebsites.net
Purpose
Try the functionality out here.
This pull request fixes #37 by retrieving a user token from an internal Microsoft API that provisions a user and a token. It makes this available for deployment by users by issuing ARM templates for deployment to Azure.
Connection to Token-providing API
When the server is hosted in non-development environments, it cannot rely on the capabilities of the dev server to connect it to a user-proviced Azure Communications Resource. Therefore, when the front-end has to provision a user, it first tries to access any local functionality. When that fails, it tries to access https://calling-example-er.azurewebsites.net as API to provide it with a user and a token.
I advise against merging this PR. The alternative to this is to embed a small local server, like proposed in #63. In that case, the user can link the application to their own Azure Communication Service resource. The approach taken in the current PR has the following disadvantages:
ARM Templates
To facilitate deployment to Azure, an ARM template has been added. This template was built using a Bicep template which can be transpiled to the JSON template using
az bicep build --file <filename>
. The ARM template creates an App Service that hosts the code.One can test these ARM templates in Azure here:
Does this introduce a breaking change?
Pull Request Type
What kind of change does this Pull Request introduce?
How to Test
To test the deployment
deploy/azuredeploy.json
What to Check
Verify that the following are valid
Other Information