bluesky / tiled

API to structured data
https://blueskyproject.io/tiled
BSD 3-Clause "New" or "Revised" License
59 stars 49 forks source link

Cannot search by time range, Internal Server error #367

Open jilavsky opened 1 year ago

jilavsky commented 1 year ago

I am trying to search by time range defined as: http://usaxscontrol:8000/api/v1/node/search/20idb_usaxs?page[offset]=00&page[limit]=100&filter[time_range][condition][since]=1668816000&filter[time_range][condition][until]=1668902400&sort=time Server reports : Internal Server Error and log shows:

 tail logfile.txt 
    response = await func(request)
  File "/home/beams11/USAXS/micromamba/envs/tiled/lib/python3.10/site-packages/fastapi/routing.py", line 235, in app
    raw_response = await run_endpoint_function(
  File "/home/beams11/USAXS/micromamba/envs/tiled/lib/python3.10/site-packages/fastapi/routing.py", line 161, in run_endpoint_function
    return await dependant.call(**values)
  File "/home/beams11/USAXS/micromamba/envs/tiled/lib/python3.10/site-packages/tiled/server/router.py", line 176, in node_search
    resource, metadata_stale_at, must_revalidate = construct_entries_response(
  File "/home/beams11/USAXS/micromamba/envs/tiled/lib/python3.10/site-packages/tiled/server/core.py", line 153, in construct_entries_response
    query = query_class.decode(**parameters)
TypeError: TimeRange.decode() missing 1 required keyword-only argument: 'timezone'

This is using current version 0.1.0a79 OAS3. This query was created from Swagger UI.

jilavsky commented 1 year ago

Same problem with 0.1.0a80

danielballan commented 1 year ago

Thanks for the clear report, @jilavsky, and for your patience with my delayed reply.

I can reproduce this on tiled-demo.blueskyproject.io (running v0.1.0a80).

This request, based on yours but using our example data instead of USAXS data, returns 500 Internal Server Error: https://tiled-demo.blueskyproject.io/api/v1/node/search/bmm/raw?page[offset]=00&page[limit]=100&filter[time_range][condition][since]=1668816000&filter[time_range][condition][until]=1668902400sort=time

The server should reply 400 Bad Request with some details about what's missing --- i.e. a timezone parameter. That's a bug in Tiled that we can fix.

If I insert a timezone parameter &filter[time_range][condition][timezone]=US/Central I get a valid response.

https://tiled-demo.blueskyproject.io/api/v1/node/search/bmm/raw?page[offset]=00&page[limit]=100&filter[time_range][condition][since]=1668816000&filter[time_range][condition][until]=1668902400&filter[time_range][condition][timezone]=US/Central&sort=time

(In this case it's an empty results set, but presumably the same filter parameters on http://usaxscontrol:8000/api/v1/node/search/20idb_usaxs may return non-empty results.)

In the Swagger UI, I can reproduce this by omitted the timezone parameter. Observe the 500 Internal Server Error at the bottom.

image

But when I fill in a timezone it works.

image

danielballan commented 1 year ago

Note: In my post of I accidentally pasted in the broken screenshot again. I edited it to fix.

Outside the view of the screenshot, note that I set the path parameter to /bmm/raw in this example.

jilavsky commented 1 year ago

Hm, thanks. This works, checked it. Thanks!


But, the time provided is already UTC, I had to convert to UTC when originally writing my code for prior Tiled version months ago. Why is system not defaulting to UTC when no time zone is presented (as before), is there any solid reasoning?

In longer run a page with a good explanation of web query options is needed. Examples of queries would be probably more useful. May be link to some introductory page how to build the queries would be helpful. Existing help I can find: https://blueskyproject.io/tiled/reference/queries.html and https://swagger.io/docs/specification/about/ are difficult to turn into meaningful queries.

danielballan commented 1 year ago

I don't recall intentionally removing the UTC default. That was probably an accidental change. I would be happy to reinstate the UTC default in a future release.

Yes, I definitely agree about documentation. I'd like to address this in two ways:

danielballan commented 1 year ago

Issue for improving query docs: https://github.com/bluesky/tiled/issues/373

I'm second-guessing that statement about timezones though. I could see users coming in with different expectations:

Forcing the client to be specific about the timezone seems prudent, as long as the error message if they forget to do so is very clear. Users won't generally be forming these URLs by hand, so a given application like your Igor code can of course hard-code some timezone or other. Does that seem reasonable?

jilavsky commented 1 year ago

I understand different expectations. My view is that it should be UTC and that should be clearly stated in the manual -- there are limits to helpfulness. And you are right, if timezone is required, Igor code has no problem attaching the timezone once I know about this requirement. My bigger issue is with webUI, where user needs to add this field or gets error - web interface does not seem smart enough to require timezone selection when needed. And then we get to the simple problem of "what is the time zone code?" I have no clue, which means I need to figure out it is needed and then figure out what the proper syntax is by web search. All while web UI is returning error.

danielballan commented 1 year ago

That makes sense. I think we can straightforwardly make that into a select box, to start, and improve further from there.