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

Purge WRDS API Dependency from Viz Pipelines #810

Merged
merged 5 commits into from
Nov 22, 2024

Conversation

shawncrawley
Copy link
Collaborator

The changes herein originated from ticket #772 - namely improving the process that synchronizes the on-prem WRDS location DB with our cloud version. The syncing process started failing a couple of months back. Turns out that was due to permission errors on the WRDS-side when trying to write the Location DB dump to S3. So with the help of @DrixTabligan-NOAA, we went ahead and tackled #590 to get the DB dump uploaded to the NWM Shared bucket instead.

Once #590 was complete, the syncing process continued to fail due to the WRDS API test failing on the new location DB dumps. This is expected behavior, since the most recent changes to the WRDS DB were more significant. However, rather than just fix the failing code, I also completely removed the dependency on the WRDS API (since none of our services depend on it anymore) and replaced it with tests that are DB-centric.

The terraform code that creates the Test WRDS DB lambda packages every SQL file across our whole repository into a folder that the lambda has access to. When the tests run, the code creates a foreign db connection to the wrds_location3_ondeck db via a new test_external schema and also creates a automated_test schema. Then the code iterates through every SQL file and checks if it has any dependency on the WRDS db (i.e. references to the external schema). If the dependency exists, the SQL is tweaked on the fly to access the wrds_location3_ondeck db via the test_external schema, and also to write any intermediate SELECT ... INTO results to the automated_test db instead. If all of the relevant SQL files execute successfully, the tests pass and the wrds_lcoation3_ondeck db is swapped for the live (i.e. non-_ondeck) version.

That's that! No more dependence upon the WRDS API and even more robust and relevant testing.

@shawncrawley
Copy link
Collaborator Author

We need to get this in ASAP. The WRDS DB syncing has been failing for months again since the initial changes were put in place manually. A VLab ticket was just opened a few days ago pointing out that our DB is out of sync: https://vlab.noaa.gov/redmine/issues/136423.

@DrixTabligan-NOAA Let's tag up on this.

@shawncrawley shawncrawley marked this pull request as ready for review November 19, 2024 18:45
Copy link
Collaborator

@DrixTabligan-NOAA DrixTabligan-NOAA left a comment

Choose a reason for hiding this comment

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

Looks good.

@DrixTabligan-NOAA DrixTabligan-NOAA merged commit ea8b7ae into ti Nov 22, 2024
1 check passed
@DrixTabligan-NOAA DrixTabligan-NOAA deleted the purge-wrds-api-dependency branch November 22, 2024 17:57
shawncrawley added a commit that referenced this pull request Nov 27, 2024
DrixTabligan-NOAA pushed a commit that referenced this pull request Nov 27, 2024
Refs #772

There were a number of issues that had to be ironed out after merging
and deploying PR #810. All of the fixes herein have been fully tested
via a deploy to `ti` and manual triggers of the resources.
shawncrawley added a commit that referenced this pull request Dec 11, 2024
@nickchadwick-noaa nickchadwick-noaa added this to the V2.1.8 milestone Jan 28, 2025
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.

3 participants