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

Datetime no longer a parameter option in GET /search #315

Closed
zstatmanweil opened this issue Jan 7, 2022 · 3 comments · Fixed by #318
Closed

Datetime no longer a parameter option in GET /search #315

zstatmanweil opened this issue Jan 7, 2022 · 3 comments · Fixed by #318

Comments

@zstatmanweil
Copy link
Contributor

zstatmanweil commented Jan 7, 2022

In a recent PR,Dict was added as a type option for datetime in BaseSearchGetRequest

datetime: Optional[Union[str, Dict]] = attr.ib(default=None)

After this change, for some reason datetime is no longer registering as a parameter in the GET /search endpoint for sqlalchemy or pgstac backend. You can confirm this by running it locally, and looking at the /docs or /api pages. Alternatively, you can run a GET request on this URL and see that the item returned should actually be filtered out based on the datetime:

http://0.0.0.0:8082/search?collections=joplin&ids=f2cca2a3-288b-4518-8a3e-a4492bb60b08&datetime=2021-02-02T00:00:00Z/2022-02-02T00:00:00Z

I removed the Dict option and datetime reappeared as a parameter.

Since this was made as part of the pgstac upgrade to 0.4.0, I assume it has something to do with that, but downstream all the functions expect a str or datetime so I haven't been able to figure it out. I see that cql2-json can take a dictionary for datetime within the filter parameter, but since the datetime parameter is not available in cql2-json, I don't think that applies here.

@zstatmanweil
Copy link
Contributor Author

hey @bitner , why was Dict added as an option to the datetime parameter in GET /search? I am happy to add a PR to fix this issue and add tests etc. but I didn't want to just delete that without understanding!

@bitner
Copy link
Collaborator

bitner commented Jan 10, 2022

Hey @zstatmanweil off the top of my head, I'm not entirely sure. Definitely open to a PR with Tests!

There are definitely going to be quite a few changes in the area coming up as I'm working on CQL2-Text support. From a backend perspective, pgstac only takes cql-json (which should be considered deprecated as new work will only focus on cql2) or cql2-json, so the GET's job is just to convert the GET parameters into CQL-JSON / CQL2-JSON. I'm currently adding at cql2-json backend to pygeofilter and the plan is that anything other than CQL2-JSON in stac-fastapi-pgstac will get translated by pygeofilter into cql2-json.

@zstatmanweil
Copy link
Contributor Author

Thanks, @bitner. All makes sense! I will open a PR for this with some additional tests to catch this issue. More tests should also help with this big overhaul - looking forward to seeing it

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 a pull request may close this issue.

2 participants