-
Notifications
You must be signed in to change notification settings - Fork 478
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
[Fabric E2E Sample] Readme update #1044
base: kitsune/ci_pipelines_
Are you sure you want to change the base?
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
1. Developers develop in fabric workspaces as their own Sandbox environments and commit changes into their own short-lived git branches. (i.e. <developer_name>/<branch_name>) | ||
2. When changes are complete, developers raise a PR to main for review. This automatically kicks-off the [PR validation pipeline](./devops/azure-pipelines-ci-qa.yml) which: | ||
- Runs the unit tests and lint checks for [python custom libraries](./libraries/). | ||
- Sets up and tests an ephemeral build workspace in Fabric, requiring **interactive Azure CLI login**. It creates necessary resources like a feature workspace, custom work pool, ADLS Gen2 storage container, ADLS Gen2 Cloud connections and ADLS Shortcut. The workspace are synced to the feature git branch. These resources are created per PR and reused if they already exist. The pipeline publishes compute settings and libraries as needed, re-publishing the environment if there are changes for the environment config files or custom libraries in the PR. It also creates config files for the solution and uploads them to the ADLS Gen2 container, finally running a notebook to verify the setup. |
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.
- 'requiring interactive Azure CLI login. '
I wonder if we can run it automatically as in the pipeline we may not be able to run CLI manually - 'It creates necessary resources like'
It will be good if we can have a list about what resource are created - 're-publishing the environment if there are changes for the environment config files or custom libraries in the PR.'
It will be checked by the script? - 'finally running a notebook to verify the setup.'
Please specify the name of notebook. And is it to validate the setup or the pipeline?
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.
We need to check where we should put this document. Please create a separate document in the PR first.
@@ -43,6 +43,53 @@ The diagram below illustrates the complete end-to-end CI/CD process: | |||
|
|||
![Fabric CI/CD diagram](./images/fabric-cicd-option1.png) | |||
|
|||
1. Developers develop in fabric workspaces as their own Sandbox environments and commit changes into their own short-lived git branches. (i.e. <developer_name>/<branch_name>) |
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.
These are just quick comments:
-
Instead of this outline:
- Description of all pipelines
- Testing
- Cleanup.
A good outline would be:
- CI (PR Validation)
- CI (Build Artifacts)
- CD (Release and deploy)
-
There are parts where it is too specific when it doesn't need to be. And some parts that is not specific enough. I think a good way to go about it is:
- a brief description of what the pipeline does
- a brief explanation of the broad steps. Specific details are already in the code.
- an explanation of why we chose to do it this way.
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 will follow up on examples of this.
Type of PR
Purpose
Update README for CI pipeline
Investigated #1013, summarizing them into a simple yet detailed document. The writing style follows the one used by Databricks. However, I have included more detailed explanations to clarify what happens in the pipeline, ensuring it is not overly lengthy.
Document Updated
NOTE
Compared to the Databricks documentation, the following document might also be necessary. However, since the CD pipeline is not yet complete, these updates are not included in this PR.
Does this introduce a breaking change? If yes, details on what can break
No
Author pre-publish checklist
Validation steps
Issues Closed or Referenced