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

Replace lambda function to update city_id #1253

Merged
merged 7 commits into from
Jul 26, 2023

Conversation

roseeichelmann
Copy link
Contributor

@roseeichelmann roseeichelmann commented Jul 14, 2023

Associated issues

Closes cityofaustin/atd-data-tech#12744

This PR addresses the other part of what the lambda function does (first part was addressed here #1247) which is updating the city_id to be in Austin when a crashes position is updated to be inside of Austin. I tested the current set up and the lambda function is not doing this as it should be.

To determine whether city_id = 22 (Austin) im using the same jurisdictions that the OG lambda functions use, which is (5, 3, 7, 8, 10).

image

Testing

URL to test:
Test staging

Steps to test:

  1. Go to a crash, change the city to be outside of Austin on this Details card.
    image
  2. Now edit the crash coordinates to be somewhere inside of Austin.
  3. Refresh the page, the City should now say Austin in the place you originally edited (aka city_id was updated to 22).

Steps for merge:


Ship list

  • Code reviewed
  • Product manager approved

Copy link
Member

@frankhereford frankhereford left a comment

Choose a reason for hiding this comment

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

This is powerful work Rose! You're bringing VZ lightyears ahead step by step with these optimizations and restructuring how the data is managed.

Interestingly, following your directions, I don't even have to refresh the page to get the reassigned city name after the trigger runs. It's just 💯 🎯.

Copy link
Member

@patrickm02L patrickm02L left a comment

Choose a reason for hiding this comment

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

Testing

  • 1. Go to a crash, change the city to be outside of Austin on this Details card.
    image
  • 2. Now edit the crash coordinates to be somewhere inside of Austin.
  • 3. Refresh the page, the City should now say Austin in the place you originally edited (aka city_id was updated to 22).

Notes

This is really cool! 🕶️

Same observation as Frank, I didn't need to refresh it happened automatically.

No sound on vid
https://github.com/cityofaustin/atd-vz-data/assets/5566727/2c53b554-4f8b-4ff7-bbe3-fa99074671d2

FROM atd_txdot_crashes AS crashes
INNER JOIN atd_jurisdictions jurisdictions
ON (jurisdictions.geometry && NEW.position) AND ST_Contains(jurisdictions.geometry, NEW.position)
WHERE crashes.crash_id = NEW.crash_id
Copy link
Member

@johnclary johnclary Jul 24, 2023

Choose a reason for hiding this comment

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

👋 just eyeballing this sql, @roseeichelmann can you help me understand why it is necessary to join to atd_txdot_crashes? it looks to me like you only need to select from atd_jurisdictions to get the info you need.

Copy link
Contributor Author

@roseeichelmann roseeichelmann Jul 26, 2023

Choose a reason for hiding this comment

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

no you are totally right, its not necessary! brain fart :p

just pushed up changes and recreated that function in staging

@johnclary
Copy link
Member

Do we need to tear down the lambda function in concert with this change? Or is the plan to just disable the hasura event trigger?

@roseeichelmann
Copy link
Contributor Author

@johnclary i was thinking we would disable the hasura event trigger and then tear down the lambda as well, but curious what others think

@roseeichelmann roseeichelmann merged commit c5000cb into master Jul 26, 2023
8 checks passed
@roseeichelmann roseeichelmann deleted the 12744_replace_lambda_city_id branch July 26, 2023 17:12
@mddilley
Copy link
Contributor

@roseeichelmann @johnclary I think that tearing down the related lambdas and sqs AWS resources is a great idea for a spin-off issue unless someone really thinks that it should happen now.

I quickly looked at the code/CI/AWS resources, and I'm reading that the CI will continue to deploy lambdas and SQS for each folder in the atd-vz-data/atd-events. So, they will keep coming back until that code is removed too.

@roseeichelmann
Copy link
Contributor Author

@mddilley okay that sounds like a good plan to me, ill bring this up in planning today as a potential spin-off issue to create

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace lambda function to update city_id
5 participants