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

Brsherwi/829 #986

Open
wants to merge 9 commits into
base: e2e/databricks/parking-sensors-V1
Choose a base branch
from
Open

Conversation

bsherwin
Copy link
Contributor

  • Documentation changes
  • Code changes
  • Test changes

Purpose

This PR removes all references to the Melbourne Parking Sensor data and replaces them with fictional data. This includes pipelines, datasets, linked services, and documentation.

Author pre-publish checklist

  • No PII in logs
  • Made corresponding changes to the documentation

Validation steps

After running the ./deploy.sh the application should be deployed as before. The ADF should point to the deployed app service and the data should be able to be pulled in batch from the app service application.

Issues Closed or Referenced

@bsherwin bsherwin requested a review from ydaponte December 19, 2024 22:42
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.

There are some changes to be addressed. Please ping me when you start your day and we can discuss the details if you want. Thank you!

Copy link
Contributor

@elenaterenzi elenaterenzi left a comment

Choose a reason for hiding this comment

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

You'll have to rebase/merge your branch with the new version of the feature branch (you have merge conflicts), other than that see my comments.
I haven't finished reviewing all files in the PR, waiting for your feedback before I continue with my review.

e2e_samples/parking_sensors/scripts/deploy_appservice.sh Outdated Show resolved Hide resolved
Copy link
Contributor

@elenaterenzi elenaterenzi Dec 20, 2024

Choose a reason for hiding this comment

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

the zip file contains the actual web app code, and like this (as it's a zip) we cannot review its contents nor do source control. Plus we might need to change the contents in the future, so we need to host the web app code "in clear text" on the repo and via deploy_infrastructure.sh script compile it and ship it to appservice. Basically, we need a step prior to ARM deployment where we compile and create the zip file which is then shipped to appservice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For now, can we leave this as a zip, until we decide how the data-simulator is going to be source controlled in the overall solution? Making this change will involve updating the prerequisites for the user to have nodejs and a command-line zip installed on their machine (and verify in devcontainer) in order do this during deployment time.

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