Rr 115 update docs#138
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the documentation to improve onboarding/troubleshooting guidance and reflect newer client capabilities in cfa-cloudops.
Changes:
- Add new troubleshooting sections for CloudClient instantiation and file-path/mount issues during jobs
- Expand the CloudClient getting-started notebook with clearer guidance around env files, blob containers, ACR uploads, pools, and jobs
- Update docs to mention
ContainerAppClientand adjust the service-principal sample.env
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/troubleshooting.md | Adds new troubleshooting guidance for CloudClient instantiation and file-not-found issues. |
| docs/overview.md | Mentions ContainerAppClient as a repo component. |
| docs/files/sp_sample.env | Removes AZURE_SP_CLIENT_ID from the SP sample env. |
| docs/examples/getting_started/cloudclient_walkthrough.ipynb | Expands the getting-started walkthrough with additional explanations and examples. |
| docs/examples/getting_started/Dockerfile | Replaces the minimal Dockerfile with a more explanatory example. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
sfrosenb
left a comment
sfrosenb
left a comment
There was a problem hiding this comment.
Cloudops Comments Not Directly Addressed by PR Suggestions
Overview
Several members of my team spent significant time (over 2 weeks) debugging the walkthrough for this package before we could get it to work, and some of us are still encountering issues. This seems to point to some underlying difficulties with CFA–Azure interaction or with the package itself. Below are a few suggestions that may help address these challenges.
Stress‑Test Functions
Several cloudops functions are somewhat brittle, failing under certain assumptions that fall outside typical user workflows.
Cc.monitor_task(download_task_output = True)
- This function can fail if any tasks associated with a job no longer have nodes.
- Possible improvements:
- Would it be possible to add a try–catch or filtering step to ignore tasks lacking nodes?
- Alternatively, a more informative error message could help users understand when a new job is required.
- If helpful, I’m happy to submit a feature request.
Cc.create_job
- If the pool does not exist, the job appears to be created, but tasks submitted to it remain queued indefinitely.
Pre‑Identify User Issues
- Reviewing feature requests from the predecessor package (
cfa-azure) may help proactively address common user concerns.
Improve Error Messages
Cc.monitor_taskfails when tasks lack associated nodes, but the resulting error is not very informative.
Reduce Debugging Time
Debugging has been more challenging because—even with vmsize = 'xsmall'—the transition from queued → running typically takes at least 4 minutes.
Possible improvements:
- Is there any possibility of creating a pool that allows for faster debugging?
- Would dedicated nodes help? Or would they pose too high a cost risk if users forget to delete the pool?
- If startup time cannot be reduced, making it easier to launch multiple tasks for testing might help:
- Currently, if one task fails earlier than another, the second call to
download_task_outputerrors out.
- Currently, if one task fails earlier than another, the second call to
Other Suggestions
- In the walkthrough, it could be more helpful for users if argument names were explicitly specified rather than relying on positional interpretation.
| "Pools are usually created for each team or per project. It spins up nodes when necessary based on the container you specify. The following would create a pool based on the Docker image we just uploaded, autoscaling to 5 nodes, mounting to the 'input-test' container we uploaded to, an 8 core CPU, and call it 'getting-started-pool'. " | ||
| "Pools are usually created for each team or per project. It spins up nodes when necessary based on the container you specify. \n", | ||
| "\n", | ||
| "It's at this point we specify which Blob Containers we mount to the pool. This will make blobs in Blob Storage accessible to read or write for the containers that we mount. The mounts are then accessible in your code at the root of the node, i.e. a mounted container called 'input-test' will be accessible in your code via `/input-test`. \n", |
There was a problem hiding this comment.
Access to mounted files continues to be an issue for my team. It is unclear as of yet what is going on, but in some cases, tasks fail access the files in the blob container mounted to the pool
There was a problem hiding this comment.
several members of my team work almost exclusively in R, so it was important to establish the ability to run simple R scripts rather than simple python scripts. As such, to test this I created a simple dockerfile and corresponding helloworld script that you could consider including in this PR, or I can include in my own PR
There was a problem hiding this comment.
If you want to share that with me I can definitely include it. That would be great. Thanks
|
I think adding clarity around the dockerfile beyond pointing to the generic docker docs would help users and troubleshooting. Your example dockerfile sets WORKDIR to /app but then in the walkthrough, "app" is not used in create_pool() nor the add_task() nor main.py, and docs/troubleshooting.md does not use it either. It would be helpful to provide the user an example Dockerfile that works with the walkthrough |
| "\n", | ||
| "The following would create a pool based on the Docker image we just uploaded, autoscaling to 5 nodes, mounting to the 'input-test' container to which we uploaded, use an 8 core CPU, and call it 'getting-started-pool'. \n", | ||
| "\n", | ||
| "You could also specify vm_size from a list of xsmall, small, medium, large, and xlarge. These will use 2, 4, 8, 16, or 32 cores, respectively." |
There was a problem hiding this comment.
Can you add more information about what the various arguments for create_pool do? In the walkthrough you have max_autoscale_nodes=5. But when I tried this out my job did not seem to autoscale like it used to with out old cfa-azure pipeline. Does autoscale need to be set to true? Whats going on?
No description provided.