feat: add transducer observation handling and improve database initialization

[jirhiker]

Why

This PR addresses the following problem / context:

• Use bullet points here

How

Implementation summary - the following was changed / added / removed:

• Use bullet points here

Notes

Any special considerations, workarounds, or follow-up work to note?

• Use bullet points here

[codecov-commenter]

Codecov Report

❌ Patch coverage is 53.33333% with 7 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
services/observation_helper.py	0.00%	5 Missing ⚠️
api/observation.py	50.00%	1 Missing ⚠️
core/app.py	50.00%	1 Missing ⚠️

Files with missing lines	Coverage Δ
core/initializers.py	87.65% <100.00%> (+1.63%)	⬆️
tests/test_observation.py	94.13% <100.00%> (-1.70%)	⬇️
api/observation.py	96.00% <50.00%> (-4.00%)	⬇️
core/app.py	29.62% <50.00%> (-0.50%)	⬇️
services/observation_helper.py	76.66% <0.00%> (-15.38%)	⬇️

... and 16 files with indirect coverage changes

[jacob-a-brown]

This raises a 404 error if thing_id does not exist, but since thing_id is a query parameter if there are no observations for that thing (one of the reasons could be that the thing_id does not exist), it should just return an empty list.

I think that 404 errors should only be returned for path parameters since the full path is a resource.

[jirhiker]

That is not how the feature file is written. https://github.com/DataIntegrationGroup/OcotilloBDD/blob/main/features/backend/transducer-data-response.feature

When the user requests transducer data for a non-existing well
      Then the system should return a 404 status code

[jirhiker]

Remaining faithful to the feature file, it seems like making thing_id a path parameter is the best approach. In the case of using a query parameter, I don't think it makes sense to get an empty list if the thing_id is invalid.

I think internal int ids should be path parameters
get observation/transducer-groundwater-level/123

Human-readable identifiers like name, pointid, site-name, etc, should be query parameters. e.g.

get observation/transducer-groundwater-level?thing.name == 'WL-001'
but we don't have a need for this functionality at this time.

[jacob-a-brown]

I wrote up a discussion about this a while ago to record my thoughts: https://github.com/orgs/DataIntegrationGroup/discussions/14

The empty list vs 404 error came up when @TylerAdamMartinez and I were working on AMP API and the water level entry form a while ago. I can look more into the appropriate response when a user supplies an erroneous query parameter.

I also agree that path parameter ids should pertain to that table

[jirhiker]

@jacob-a-brown can we get this approved and merged

[jacob-a-brown]

Would you be okay to change the feature file to return an empty list when a thing_id is provided as a query parameter but it doesn't exist in the database? Then filter by thing_id instead of using simple_get_by_id, which raises a 404 error. From what I've read that's the standard practice.

https://softwareengineering.stackexchange.com/questions/203492/when-to-use-http-status-code-404-in-an-api
https://stackoverflow.com/questions/30217761/http-200-or-404-for-empty-list
https://www.reddit.com/r/learnprogramming/comments/1lirdwc/for_a_rest_api_fetch_with_parameters_should_you/
https://www.servicenow.com/community/developer-forum/404-error-in-case-of-rest-api-not-returning-any-data/m-p/3087325
https://chatgpt.com/share/e/6915fe67-288c-800a-a9c7-bef9f0c548c8

[jirhiker]

I'm good with that pattern for other endpoints. I removed the functionality of using thing_id as a query parameter for transducer observations for this iteration as there isn't a use case for it yet

[jacob-a-brown]

I missed that, I was still reading the thing_id as a query parameter. Does it make sense to have thing_id be the path parameter for a table for which it is not the ID? I thought that the IDs for path parameters pertained to the table itself, not related tables

[jirhiker]

It's a good point. It's not something that matters in practice but for consistency I will change back to a query parameter