Skip to content
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

[Databricks E2E] Fix devcontainer for wheel library builds #1040

Merged

Conversation

bsherwin
Copy link
Contributor

Type of PR

  • Code changes

Purpose

Fix the ddo_transform devcontainer to support building the wheel for dev deployments. Refresh JavaSDK.

Does this introduce a breaking change? If yes, details on what can break

No

Validation steps

  • Follow build steps in the src/ddo_transform/readme.md to run make dist

Issues Closed or Referenced

@bsherwin
Copy link
Contributor Author

@ydaponte and @raafatzarka - I recreated this PR since the previous PR included more changes than I had expected. Sorry for the oversight.

@raafatzarka raafatzarka self-requested a review January 15, 2025 20:07
@raafatzarka
Copy link
Contributor

Per our pairing session testing this code in Mac, please add the .env file and it seems to be working fine

Copy link
Contributor

@raafatzarka raafatzarka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once the .env file is added, it is approved

@bsherwin
Copy link
Contributor Author

bsherwin commented Jan 15, 2025

.env files are normally ignored with .gitignore, but since it was causing an error, we added it to the repo. Normally, this is empty unless you are using the make file to publish to DBX.

One more PC review would be good.

@bsherwin bsherwin self-assigned this Jan 15, 2025
Copy link
Collaborator

@ydaponte ydaponte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bsherwin - the REST API PR already has this changes, is this PR then necessary as well? Or was it just to facilitate the testing? I've duplicated some of the comments I did on the other PR, but if you want to keep them separated, maybe you remove the changes from the other PR? I leave it up to you.

@bsherwin
Copy link
Contributor Author

@bsherwin - the REST API PR already has this changes, is this PR then necessary as well? Or was it just to facilitate the testing? I've duplicated some of the comments I did on the other PR, but if you want to keep them separated, maybe you remove the changes from the other PR? I leave it up to you.

Yes...this is what will happen with overlapping PRs. Once this is merged to V1, the merge will remove it from the other PR (or merge silently).

@raafatzarka
Copy link
Contributor

Tested again on Mac with the new changes and it works fine when running the command "make dist"

@bsherwin
Copy link
Contributor Author

@ydaponte Is this good to go?

@bsherwin bsherwin merged commit 2498f48 into e2e/databricks/parking-sensors-V1 Jan 30, 2025
3 checks passed
@bsherwin bsherwin deleted the brsherwi/ddotransformcontainer branch January 30, 2025 17:04
elenaterenzi added a commit that referenced this pull request Feb 6, 2025
* [E2E Databricks] - Automate region vm + dbx type cluster in the region (#988)

* automate region vm + dbx type cluster in the region

* Update e2e_samples/parking_sensors/scripts/configure_databricks.sh

Co-authored-by: Elena Terenzi <[email protected]>

* filtering photon support

---------

Co-authored-by: Elena Terenzi <[email protected]>

* [Databricks E2E Sample] - bug fix dbx compute (#1006)

* fix breaking change from dbx cli output
* fix hyperlink check

* Cluster config update (#1012)

* Update clean_up.sh

Adding the Cleanup for federal credentials which follows a certain order:
Get the SP Objc ID of the Service Connection
Delete federated credentials from the SP Object ID
Delete the service connection

* Update deploy_azdo_service_connections_azure.sh

Service Connections are using the Workload Identity which comes from a configuration file.

The cleanup in case of re-start of this script also follows a peculiar order: 

Get SP object if according to the Service Connection
Get the federate credential according to the SP
Delete federated credentials
Delete the Service Connection

* Update README.md

changing the docs with the required permission

* Update README.md

rephrase the permissions needed for azdevops project

* Update e2e_samples/parking_sensors/scripts/clean_up.sh

Co-authored-by: Yennifer Santos <[email protected]>

* Update e2e_samples/parking_sensors/scripts/deploy_azdo_service_connections_azure.sh

Co-authored-by: Yennifer Santos <[email protected]>

* Update README.md

I adjust the order of the permissions

* Update README.md

adding know issues how to delete federate credentials

* Update README.md

identation was not correct

* Fix The property "options" Warnings in Dashboard.bicep

* Update deploy_azdo_service_connections_azure.sh

adding the following changes:
1) Function for cleanup
2)Function for sleep
3)Remove log file

* Update clean_up.sh

Implement function for cleanup and function for sleep

* Update common.sh

Added cleanup_federate_credentials and wait_for_credentials used in the clean_up.sh and deploy_azdo_service_connection.sh

* Update clean_up.sh

rename wait_for_cleanup to wait_for_process

* Update deploy_azdo_service_connections_azure.sh

renamed wait_for_cleanup to wait_for_process

* Fix typo in clean_up.sh log message

typo:
comtain to contain fxed

* Update e2e_samples/parking_sensors/README.md

added Elena's suggestion of rewrite

Co-authored-by: Elena Terenzi <[email protected]>

* Update common.sh

remove the log for SP

* Update deploy_azdo_service_connections_azure.sh

Clean up of the log
suppress update pipelines message
remove response from logging

* Update common.sh

changing the log for federate credentials removal

* Update common.sh

adding the SP in the log message

* Update deploy_azdo_service_connections_azure.sh

checking if the file exist before removal.

* Update e2e_samples/parking_sensors/scripts/common.sh

Co-authored-by: Elena Terenzi <[email protected]>

* Update e2e_samples/parking_sensors/scripts/common.sh

Co-authored-by: Elena Terenzi <[email protected]>

* Update e2e_samples/parking_sensors/scripts/common.sh

Co-authored-by: Elena Terenzi <[email protected]>

* Update e2e_samples/parking_sensors/scripts/deploy_azdo_service_connections_azure.sh

Co-authored-by: Elena Terenzi <[email protected]>

* Update e2e_samples/parking_sensors/scripts/deploy_azdo_service_connections_azure.sh

Co-authored-by: Elena Terenzi <[email protected]>

* Update e2e_samples/parking_sensors/scripts/clean_up.sh

Co-authored-by: Elena Terenzi <[email protected]>

* Update e2e_samples/parking_sensors/scripts/common.sh

Co-authored-by: Elena Terenzi <[email protected]>

* Update e2e_samples/parking_sensors/scripts/common.sh

Co-authored-by: Elena Terenzi <[email protected]>

* Remove unnesissary dependsOn in main.bicep

* Remove redundant comment and variable

role unecessary

* Checking if I can solve the conflicts

* conflict

* conflict

* The new file after tryto solve the conflict

* Trying again to solve the conflict

* Suppress output for service connection deletion

adding -0 none to resolve conflict

* add code to delete app registrations (#1019)

* [Databricks E2E] [BUG] cleanup fix bug with federated credential removal (#1047)

* remove federated credentials only when needed

* fix typo

* [Databricks E2E]: Remove AzDO Service Connection Principal (#1057)

* remove SPN for AzDO service connection

* minor edits

* minor edits

* minor typo

* add readme change for manual deletion

* Deploy.sh will have the option to choose deployment type (#1049)

* Deploy.sh will have the option to deploy as:
1) dev
2) Dev and Stage
3) Dev, Stage and Prod

Common.sh will heold the function for this changes
envtemplate the new environmnent variable for the
deployment options
deploy_azdo_pipelines.sh will deploy the release
pipeline if option 2 or 3 are choosen which
implies the deploy of t least 2 environmnents

* adjusting the readme according to the latest version in v1

* Seems this session was not veru well documented -
**Workaround - Manually Deleting Federated Credentials:**

* to* typo

* Update e2e_samples/parking_sensors/README.md

Co-authored-by: Copilot <[email protected]>

* Remove duplicate deployment instruction in README

fixing copilot fix

* Adding if for deploy.sh respect configuration in
the enviromnent variable

Initialize env variable in the init_environment

* adjust the readme for env. varaible configuration

* Clarify deployment options and environment variable usage

Adjusting explanations about env variable in the readme and envtemplate

* Update workaround for deleting Azure DevOps Service Connection

fixing conflict

* Refactor cleanup function to delete service principal

fix the conflict

---------

Co-authored-by: Copilot <[email protected]>

* [Databricks E2E] Redesign CI CD Process diagrams with drawio (#1056)

* Recreates the CI CD process diagrams in drawio
* Remove the ADF publish step from CI CD process diagrams
* Update the README.md to reflect the changes in the process sequence and diagram files

---------

Co-authored-by: Jose Perales <[email protected]>

* update SQL Pool definition to avoid resizing on redeployment (#1066)

* [Databricks E2E] Fix devcontainer for wheel library builds (#1040)

* Fix devcontainer for wheel library builds
* env file required for devcontainer
* Update e2e_samples/parking_sensors/src/ddo_transform/.devcontainer/Dockerfile

* [E2E Databricks] - fix clusterId and release folders (#1064)

* fix: release paths and cluster id

* fix: adding azdo file

* fix: removing unnecessary config

* update: readMe, optimize code flow

* update: upgrade DB CLI and fix release pipeline

* fix: pr comments

* fix: py extension on adf pipeline (#1072)

* [Databricks E2E] Rzarka/882 synapse pause resume (#1060)

* Add PowerShell and Bash scripts for Synapse Pause/Resume

* Support Manual and Automated Synapse Pause PowerShell Script with Additional features

* Add README file for Synapse Pause script

* Add References section to Synapse Pause README file

* Add InstallModules parameter to pause_resume_synapse script for automatic module installation

* Enhance README and script usage documentation for pause_resume_synapse and improved logging for state transitions

* Update README_pause_resume_synapse to include example values for parameters

* Update README_pause_resume_synapse to provide specific examples for ResourceGroups parameter

* small fix

---------

Co-authored-by: Yennifer Santos <[email protected]>
Co-authored-by: Ayman El-Ghazali <[email protected]>
Co-authored-by: LiliamLeme <[email protected]>
Co-authored-by: Raafat Zarka <[email protected]>
Co-authored-by: Liliam Leme <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Jose Perales <[email protected]>
Co-authored-by: Jose Perales <[email protected]>
Co-authored-by: Brian Sherwin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants