-
Notifications
You must be signed in to change notification settings - Fork 485
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] Merging version v0.2 #961
Conversation
* docs(e2e-fabric): add known limitations and workarounds * docs(e2e-fabric): add known limitations and workarounds * Update e2e_samples/fabric_dataops_sample/README.md Co-authored-by: Anuj Parashar <[email protected]> * docs: change to snakecase, move to docs folder * docs: change to snakecase, move to docs folder --------- Co-authored-by: Anuj Parashar <[email protected]>
…com/Azure-Samples/modern-data-warehouse-dataops into feat/e2e-fabric-dataops-sample-v0-2
feat(e2e_samples): add ADR on CI/CD workflow This ADR document the decision about the Fabric CI/CD option for milestone 1. Refs: #790 --------- Co-authored-by: Lace Lofranco <[email protected]>
fix(e2e_samples): Minor bug fixes in the deployment scripts Authored-by: Anuj Parashar <[email protected]> Reviewed-by: Naga Nandyala <[email protected]> Refs: #827, #875, #893, #872, #892
Added support for ADLS Cloud Connection creation in Fabric, updated docs and the `.envtemplate` file. --------- Co-authored-by: Jose Perales <[email protected]> Co-authored-by: Anuj Parashar <[email protected]>
Adding pre-requisite checks
) * Automating the process of creating and uploading config file to ADLS Gen2 storage account. * Automating the process of uploading seed (aka reference) data to ADLS Gen2 storage account. --------- Co-authored-by: Anuj Parashar <[email protected]>
) * Terraform related code changes * Reference to AzDo documentation updated in README.md. --------- Co-authored-by: Anuj Parashar <[email protected]>
* Updated notebooks, pipelines, and standardized folder structure; improved logging and linting. * Enhanced dim table insertion, streamlined tests, and optimized DDO module compatibility. --------- Co-authored-by: Sean Ma <[email protected]> Co-authored-by: yuna-s <[email protected]> Co-authored-by: Anuj Parashar <[email protected]> Co-authored-by: yunasugimoto <[email protected]>
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
* Multi-stage deployment changes * Documentation update
…/pipelines (#1008) Changes to notebooks to enable high concurrency execution
* Version 2.0 changes * CI changes * Notebook changes * AzDo pipelines automation * Terraform upgrade --------- Co-authored-by: Anuj Parashar <[email protected]> Co-authored-by: Lace Lofranco <[email protected]> Co-authored-by: yuna-s <[email protected]> Co-authored-by: Naga Nandyala <[email protected]>
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.
Copilot reviewed 129 out of 144 changed files in this pull request and generated 2 comments.
Files not reviewed (15)
- e2e_samples/fabric_dataops_sample/.envtemplate: Language not supported
- e2e_samples/fabric_dataops_sample/cleanup.sh: Language not supported
- e2e_samples/fabric_dataops_sample/config/application.cfg.template: Language not supported
- e2e_samples/fabric_dataops_sample/deploy.sh: Language not supported
- e2e_samples/fabric_dataops_sample/devops/deploy_infra.yml: Language not supported
- e2e_samples/fabric_dataops_sample/devops/scripts/build_adls_files.sh: Language not supported
- e2e_samples/fabric_dataops_sample/devops/scripts/requirements.txt: Language not supported
- e2e_samples/fabric_dataops_sample/docs/adr_fabric_cicd.md: Evaluated as low risk
- e2e_samples/fabric_dataops_sample/config/lakehouse_ddls.yaml: Evaluated as low risk
- e2e_samples/fabric_dataops_sample/docs/build_and_release_pipelines.md: Evaluated as low risk
- e2e_samples/fabric_dataops_sample/docs/adr_naming_convention.md: Evaluated as low risk
- e2e_samples/fabric_dataops_sample/devops/scripts/build_workspace.py: Evaluated as low risk
- e2e_samples/fabric_dataops_sample/devops/scripts/cleanup_workspace.py: Evaluated as low risk
- e2e_samples/fabric_dataops_sample/README.md: Evaluated as low risk
- e2e_samples/fabric_dataops_sample/devops/scripts/update_environment.py: Evaluated as low risk
Comments suppressed due to low confidence (3)
e2e_samples/fabric_dataops_sample/devops/templates/pipelines/azure-pipelines-ci-qa-cleanup.yml:6
- The variable PR_ID is used without being defined or passed correctly. Ensure that PR_ID is defined or passed as a parameter to avoid runtime errors.
value: "feature-$(PR_ID)"
e2e_samples/fabric_dataops_sample/devops/templates/pipelines/azure-pipelines-ci-qa.yml:159
- Include a test coverage report to ensure that the new code is adequately tested. Consider using
pytest-cov
to generate a coverage report.
pytest -o log_cli=true --capture=no test/
e2e_samples/fabric_dataops_sample/devops/templates/jobs/publish-adls-artifacts.yml:18
- The FABRIC_BEARER_TOKEN is being set but not used in the subsequent steps. Please verify if this is intended or if the token should be used somewhere.
FABRIC_BEARER_TOKEN=$(az account get-access-token --resource https://api.fabric.microsoft.com --query accessToken -o tsv)
AZURE_STORAGE_BEARER_TOKEN=$(az account get-access-token --resource https://storage.azure.com/ --query accessToken -o tsv) | ||
|
||
# Set the tokens as pipeline variables | ||
echo "##vso[task.setvariable variable=FABRIC_BEARER_TOKEN]$FABRIC_BEARER_TOKEN" |
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 use of echo
to set pipeline variables can be problematic if the variables contain special characters. Consider using the Azure DevOps CLI
to set the variables instead.
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
export FABRIC_WORKSPACE_NAME="$FABRIC_WORKSPACE_NAME-$PR_ID" | ||
|
||
# Get the list of changed files in the current commit or PR | ||
CHANGED_FILES=$(git diff --name-only $(System.PullRequest.SourceCommitId) $(System.PullRequest.TargetCommitId)) |
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.
There is no check to ensure that git
is installed before using it. Add a check to ensure that git
is available in the environment.
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
* Improving README and know issues markdown files
* Updated notebooks to use fully qualified names (FQN) when referring to lakehouse.
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.
Tested the code.. looks good.
docs(e2e-fabric): add known limitations and workarounds
docs(e2e-fabric): add known limitations and workarounds
Update e2e_samples/fabric_dataops_sample/README.md
docs: change to snakecase, move to docs folder
docs: change to snakecase, move to docs folder
Type of PR
Purpose
Does this introduce a breaking change? If yes, details on what can break
Author pre-publish checklist
Validation steps
Issues Closed or Referenced