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

Modifies temperature, precipitation, and taspr routes to not use a wildcard for the endpoint. #445

Merged
merged 1 commit into from
Jun 3, 2024

Conversation

BobTorgerson
Copy link
Contributor

This PR expands the route definitions for temperature, precipitation, and taspr to no longer have a wildcard "catch all" start to the endpoint. Without this change, any non-existent endpoint that ends in point// or area/ was resolving to these endpoints and causing an uncaught error to appear in Varnish (HTTP 503 error).

Example failure:

https://earthmaps.io/foo/bar/point/65/-156

If you run this code, and attempt that endpoint, it correctly gives a "bad request" error message:

https://localhost:5000/foo/bar/point/65/-156

Ensure that you can still get access to the temperature, precipitation, and taspr endpoints for both lat / lon and area summaries.

https://localhost:5000/temperature/point/65.0628/-146.1627
https://localhost:5000/precipitation/point/65.0628/-146.1627
https://localhost:5000/taspr/point/65.0628/-146.1627

https://localhost:5000/temperature/area/19010208
https://localhost:5000/precipitation/area/19010208
https://localhost:5000/taspr/area/19010208

…ldcard for the endpoint. Fixes error where Varnish displays a 503 error when an endpoint doesn't exist.
Copy link
Contributor

@kyleredilla kyleredilla left a comment

Choose a reason for hiding this comment

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

Yeah I see what's going on here, looks like these changes will fix the issue. Tested with supplied links in PR and cross-checked values with production API, things look good. Approved.

Copy link
Contributor

@charparr charparr left a comment

Choose a reason for hiding this comment

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

Hey this looks good! I'm testing the suggested endpoints and a few others and not hitting any issues - they work as expected for both cases where I anticipate a 503 and where I expect to get data. This PR will make it easier for us to test / develop new endpoints because we won't get misled by the mysterious guru meditation when in reality all we did was goof route the name. Like temprature which I type more than I'd like to admit. Thanks Bob!

@charparr
Copy link
Contributor

charparr commented Jun 3, 2024

Closes #350

@charparr charparr merged commit eb7058a into main Jun 3, 2024
@charparr charparr deleted the modify_taspr_endpoints branch June 3, 2024 19:44
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