-
Notifications
You must be signed in to change notification settings - Fork 17
Container apps oss labs #133
base: main
Are you sure you want to change the base?
Conversation
|
This is an OSS Labs document, we should not be forking it into our repo. It should go into the upstream repo. Lets review it here first, but not merge here. |
|
Currently stuck on avoiding hardcoding the users GitHub username in the .json file. I've read that .json files dont accept variables. Example below. "GITHUB_USER_OR_OR": "colinmixonn", |
…variables and added link to execute prereqs
… README as a user filled variable and respective instructions. Renumbered steps
…tep to prereqs. changed numbering. Added styling throughout
UmakanthOS
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.
LGTM.
rgardler-msft
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.
This doc is not executable or testable.
Not executable because there are manual steps in the prerequisites. Not testable because a) not executable and b) no results sections to validate execution against.
| 8. Click on "serverless-python" under "Packages" on the right hand side | ||
| 9. Copy the `docker pull` command which will include the image name | ||
| 10. Update the `GITHUB_USERNAME` environment variable below with your GitHub username or organization name. | ||
| * **Press `b` and run the command `GITHUB_USERNAME="username"` to set variable.** |
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 is this? It means nothing in a published document context. Why is it here?
When run in SimDem the use of an environment variable will automatically result in the customer being prompted for a value if no value exists yet.
| Currently, Container App is in preview so it requries an extension. | ||
| ``` | ||
| az extension add --name containerapp | ||
| ``` |
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.
Where is the Results section that would make this a textable doc?
| ``` | ||
| ``` | ||
| az provider register --namespace Microsoft.OperationalInsights --wait | ||
| ``` |
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.
Results sections needed.
|
|
||
| Before you can begin you need to follow the prerequisite steps found here | ||
|
|
||
| 1. Visit https://github.com/asw101/python-fastapi-pypy |
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 a manual step and thus will not work in an executable doc environment. Exec docs need to be runnable without human interaction.
If the user is expected to grab this content then they should be given the command to grab it automatically and the execution of that command would then be included in unattended execution.
| echo $GITHUB_USERNAME | ||
| ``` | ||
|
|
||
| # Step 1 - Install Azure CLI Extension |
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 header is at the same level as all the others. Is that correct? If so then Step 1 isn't really appropriate. Stylistic comment, feel free to ignore.
I was unclear what commands to include for
azure loginso I know that needs to be included before merging into main.Runs successfully in my tests from beginning to end