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

Replacement REST API #1055

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Replacement REST API #1055

wants to merge 10 commits into from

Conversation

bsherwin
Copy link
Contributor

Type of PR

  • Documentation changes
  • Code changes

Purpose

This is a NodeJS Express based REST API that will replace the existing REST API. In the readme, you can find information on how to run locally, publish to Azure using bicep and publish to Azure with Terraform.

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

No. This is an independent utility to use as you wish for ingesting data into your solution.

Author pre-publish checklist

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

Issues Closed or Referenced

@bsherwin bsherwin added e2e: parking-sensors-databricks capability: Testing Related to Testing capability e2e: fabric Related with E2E Fabric Sample labels Jan 22, 2025
@bsherwin bsherwin self-assigned this Jan 22, 2025
Copy link
Contributor

@naga-nandyala naga-nandyala left a comment

Choose a reason for hiding this comment

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

While the application and the approach of updating sensor information is working, the data (sensor_locations and sensors ) you are using does not seem to be complete.

sensor_locaiton - master data with list of roadsegments (dimension). All I see is unique road segment id (with same description)
sensors - actual sensor information (fact info)

not sure if my understanding of the data is in correct

sql_stmt = """
select roadsegmentid, roadsegmentdescription ,count(1) from {sensor_location_sdf} group by 1,2 order by 2 desc
"""

+-------------+-----------------------------------------------------+--------+
|roadsegmentid|roadsegmentdescription                               |count(1)|
+-------------+-----------------------------------------------------+--------+
|4650         |Wythe Place between Sunnyside Avenue and Tennis Court|1       |
|3984         |Wythe Place between Sunnyside Avenue and Tennis Court|1       |
|5982         |Wythe Place between Sunnyside Avenue and Tennis Court|1       |
|3318         |Wythe Place between Sunnyside Avenue and Tennis Court|1       |
|1986         |Wythe Place between Sunnyside Avenue and Tennis Court|1       |
|2319         |Wythe Place between Sunnyside Avenue and Tennis Court|1       |
|987          |Wythe Place between Sunnyside Avenue and Tennis Court|1       |
|4983         |Wythe Place between Sunnyside Avenue and Tennis Court|1       |
|321          |Wythe Place between Sunnyside Avenue and Tennis Court|1       |
|1653         |Wythe Place between Sunnyside Avenue and Tennis Court|1       |
|4317         |Wythe Place between Sunnyside Avenue and Tennis Court|1       |
|2652         |Wythe Place between Sunnyside Avenue and Tennis Court|1       |
|3651         |Wythe Place between Sunnyside Avenue and Tennis Court|1       |
|5316         |Wythe Place between Sunnyside Avenue and Tennis Court|1       |
|1320         |Wythe Place between Sunnyside Avenue and Tennis Court|1       |
|654          |Wythe Place between Sunnyside Avenue and Tennis Court|1       |
|5649         |Wythe Place between Sunnyside Avenue and Tennis Court|1       |
|2985         |Wythe Place between Sunnyside Avenue and Tennis Court|1       |
|1796         |Wyona Street between Cropsey Avenue and Banker Street|1       |
|3794         |Wyona Street between Cropsey Avenue and Banker Street|1       |
+-------------+-----------------------------------------------------+--------+

isn't the data model
One RoadSegment has multiple Kerbside_ids. Each Kerbside having one parking sensor?

utilities/data-generator/readme.md Show resolved Hide resolved
@@ -0,0 +1,27 @@
#!/usr/bin/env bash
Copy link
Contributor

Choose a reason for hiding this comment

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

Please change the permission of this script to have execute permission.

E.g Fabric post_create has execute permssions ... but this script does not..

╭─naga@naga-ms ~/dev/ext_gitrepos/brian_datagen/modern-data-warehouse-dataops/e2e_samples/fabric_dataops_sample/.devcontainer ‹brsherwi/datagenerator›
╰─$ ls -ltra
total 24
-rwxr-xr-x  1 naga naga  689 Jan 23 10:29 post_create.sh
-rw-r--r--  1 naga naga  220 Jan 23 10:29 docker-compose.yaml
-rw-r--r--  1 naga naga 1335 Jan 23 10:29 devcontainer.json
-rw-r--r--  1 naga naga 1825 Jan 23 10:29 Dockerfile
drwxr-xr-x  2 naga naga 4096 Jan 23 10:29 .
drwxr-xr-x 10 naga naga 4096 Jan 23 10:29 ..
╭─naga@naga-ms ~/dev/ext_gitrepos/brian_datagen/modern-data-warehouse-dataops/e2e_samples/fabric_dataops_sample/.devcontainer ‹brsherwi/datagenerator›
╰─$ cd ../../../utilities/data-generator/.devcontainer
╭─naga@naga-ms ~/dev/ext_gitrepos/brian_datagen/modern-data-warehouse-dataops/utilities/data-generator/.devcontainer ‹brsherwi/datagenerator›
╰─$ ls -ltra
total 24
-rw-r--r-- 1 naga naga  861 Feb  3 08:19 post_create.sh
-rw-r--r-- 1 naga naga 1044 Feb  3 08:19 docker-compose.yml
-rw-r--r-- 1 naga naga  847 Feb  3 08:19 devcontainer.json
-rw-r--r-- 1 naga naga 1746 Feb  3 08:19 Dockerfile
drwxr-xr-x 6 naga naga 4096 Feb  3 08:19 ..
-rw-r--r-- 1 naga naga    0 Feb  5 07:29 .env
drwxr-xr-x 2 naga naga 4096 Feb  5 07:29 .
╭─naga@naga-ms ~/dev/ext_gitrepos/brian_datagen/modern-data-warehouse-dataops/utilities/data-generator/.devcontainer ‹brsherwi/datagenerator›

Hence it is failing with below error

[179581 ms] Start: Run in container: /bin/sh -c ./.devcontainer/post_create.sh
/bin/sh: 1: ./.devcontainer/post_create.sh: Permission denied
[179646 ms] postCreateCommand from devcontainer.json failed with exit code 126. Skipping any further user-provided commands.
Done. Press any key to close the terminal.


## Terraform Deployment (do not run in devcontainer because it's not set up for Docker-in-Docker)

- Execute `az login` and select your subscription if prompted
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi Brian

There are too many manual terraform override steps. (tenant id, subscription id , locatoin, rg name retrieving the outputs.)
It will be nice to create a deploy.sh/deploy_infrastructure.sh file (or similar) which reads and .env file. (similar to how it is being done in fabric e2e sample).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tenant ID is not used in the terraform. Subscription and location are really the only ones I would expect to change. These could be moved to an environment file, but not sure what value it adds for terraform given you modify environment variables or you modify the variables.tf. Please explain your reasoning for using an environment variable setup.

Copy link
Contributor

@promisinganuj promisinganuj Feb 5, 2025

Choose a reason for hiding this comment

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

@bsherwin Genreally speaking, variable.tf is a declaration file where you declare your variables and optionally provide a default value or the validations that you want to perform. Same goes for providers.tf which should use variables.

The actual runtime values should be passed using .tfvars files. Or you can just provide the runtime values at the time of plan/apply as done here -

.

and

@bsherwin
Copy link
Contributor Author

bsherwin commented Feb 5, 2025

While the application and the approach of updating sensor information is working, the data (sensor_locations and sensors ) you are using does not seem to be complete.

sensor_locaiton - master data with list of roadsegments (dimension). All I see is unique road segment id (with same description) sensors - actual sensor information (fact info)

not sure if my understanding of the data is in correct

sql_stmt = """ select roadsegmentid, roadsegmentdescription ,count(1) from {sensor_location_sdf} group by 1,2 order by 2 desc """

+-------------+-----------------------------------------------------+--------+
|roadsegmentid|roadsegmentdescription                               |count(1)|
+-------------+-----------------------------------------------------+--------+
|4650         |Wythe Place between Sunnyside Avenue and Tennis Court|1       |
|3984         |Wythe Place between Sunnyside Avenue and Tennis Court|1       |
|5982         |Wythe Place between Sunnyside Avenue and Tennis Court|1       |
|3318         |Wythe Place between Sunnyside Avenue and Tennis Court|1       |
|1986         |Wythe Place between Sunnyside Avenue and Tennis Court|1       |
|2319         |Wythe Place between Sunnyside Avenue and Tennis Court|1       |
|987          |Wythe Place between Sunnyside Avenue and Tennis Court|1       |
|4983         |Wythe Place between Sunnyside Avenue and Tennis Court|1       |
|321          |Wythe Place between Sunnyside Avenue and Tennis Court|1       |
|1653         |Wythe Place between Sunnyside Avenue and Tennis Court|1       |
|4317         |Wythe Place between Sunnyside Avenue and Tennis Court|1       |
|2652         |Wythe Place between Sunnyside Avenue and Tennis Court|1       |
|3651         |Wythe Place between Sunnyside Avenue and Tennis Court|1       |
|5316         |Wythe Place between Sunnyside Avenue and Tennis Court|1       |
|1320         |Wythe Place between Sunnyside Avenue and Tennis Court|1       |
|654          |Wythe Place between Sunnyside Avenue and Tennis Court|1       |
|5649         |Wythe Place between Sunnyside Avenue and Tennis Court|1       |
|2985         |Wythe Place between Sunnyside Avenue and Tennis Court|1       |
|1796         |Wyona Street between Cropsey Avenue and Banker Street|1       |
|3794         |Wyona Street between Cropsey Avenue and Banker Street|1       |
+-------------+-----------------------------------------------------+--------+

isn't the data model One RoadSegment has multiple Kerbside_ids. Each Kerbside having one parking sensor?

Fixed the data...there was inadvertantly a 1:1 roadsegmentid to kerbsideid. Should be more in line with what you expect.

@naga-nandyala naga-nandyala self-requested a review February 5, 2025 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
capability: Testing Related to Testing capability e2e: fabric Related with E2E Fabric Sample e2e: parking-sensors-databricks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants