wereinse commented 1 year ago

Bart's comments: 05/02/23

After a walkthrough on the labs, the following improvements were identified:

[x] Add to README: To get more familiarity with kic and other tools used in this lab, please run through the inner-loop lab here

Deploy SQL section

[x] it's not clear that we deploy SQL Server to the local K8s cluster - please add documentation to ensure this is clear.
[x] typos - hierarchical
[x] Replace "hierarchical group" with "group" - confirm with Dan / Bart. Note: The assumption is that that hierarchical groups are groups with children, and non-hierarchical groups are groups used to represent labels. On the feedback Dan Massey mention to add labels, so it may be good to keep it if that is the case. We can also omit all hierarchical/label stuff and just have bare groups mentions.
[x] I believe it's 19 Groups (the root (0) doesn't count as it's not usable)
[x] Place aliases should be on separate lines
[x] Add the 'kak' alias
[x] Replace "inventory storage" with "storage"?
[x] what is the direction for us with this comment? ○ This data service allows CRUD operations against the SQL Server database (Inventory storage) deployed from previous section. while true, the DS is the only interface to the data and uses a REST / OData API. SQL Server is an implementation detail
[x] 'kak' - add into alias section
[x] Replace "the WebV" with "WebV"
[x] Define WebV in the README with a few sentences or Create a link to WebV or Create a "Tools" section at the top that has brief explanations and links. Then within the doc, we can just link to #Tools (or whatever name). That will make maintenance a LOT easier.
[x] Explain n that the 400s and 404s are "by design" / part of the test, etc. from kic test all. Point out the "0 errors" in the summary.
[x] Move the query-res-edge md file should be under the lab, not in the root. That will give the user context and the commands will fail if DS hasn't been deployed.
[x] this should be in a separate fence with an explanation and samples for each entity: # To get the beta group id kic groups list --search beta
[x] This will continuously generate 10 requests per second - Add a short explanation about how those requests get on the dashboard.

Observability: K9s

[x] K9s is more than "just logs" - "Deploy K9s" seems more in the flow
[x] I just noticed that we use the same heading for the rest of the stack, so leave the heading for now. We can update later.
[x] Provide a few sentences from the K9s website that explains K9s in more detail.
[x] Explain what the 10 RPS on API and WebV means.
[x] Maybe move that under WebV as an "advanced" feature: Press s to Toggle AutoScroll and Press w to Toggle Wrap
[x] Also make sure to turn them off.
[ ] Click on "General / Home" at the top of the screen and select "dotnet" to see Res-Edge Data Service health metrics. We were going to add a default dashboard per feedback.

Anne's comments 05/02/23

After a walkthrough in the Res-Edge and Kustomize labs with Anne, the improvements below were identified:

General

[ ] Pre-build to speed up the repo setup on codespaces

Res-edge lab

[x] Explain what create cluster command will do.
[x] Add to the notes how to create a new alias.
[x] Explain that 404 is expected and inform Errors:0 is what we are looking for.
[x] If the lab is the first one the user will execute, explain WebV and what it provides.
[ ] Check the redirect from WebV on Grafana webv logs
[x] Add what the default dashboard is showing
[x] For the commands that will still be kept as extension of kic, explain how to extend kic or link to the documenation that explains that.

Kustomize

[x] Explain more where imdb is currently deployed. i.e: In the create application overlay section, add a note informing imdb is deployed to all clusters.
[x] Reset clusters: format the commands section to look like the other basch sections
[x] to show where the changes were applied, add information or a new command to show the yaml file differences on imdb (like git diff)

wereinse commented 1 year ago

@bartr @DanMass @madossan01 @renato-marciano @olhakonst17 @yanichka99 @ehoops-microsoft @mahomedalid - please review for completeness and there is one comment about hierarchies that Dan and Bart need to resolve together.

chaosdinosaur commented 1 year ago

Closed with #51 and #49

cse-labs / res-edge-labs

README Lab comments - #48