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

Implement file searching #31

Merged
merged 6 commits into from
Jan 29, 2024
Merged

Implement file searching #31

merged 6 commits into from
Jan 29, 2024

Conversation

JBorrow
Copy link
Member

@JBorrow JBorrow commented Jan 24, 2024

Fixes #29

@JBorrow
Copy link
Member Author

JBorrow commented Jan 24, 2024

TODO: CLI

@JBorrow
Copy link
Member Author

JBorrow commented Jan 25, 2024

CLI now done

This was linked to issues Jan 25, 2024
@JBorrow
Copy link
Member Author

JBorrow commented Jan 26, 2024

@plaplant any thoughts on returning a 404 when we find no files v.s. just returning an empty list?

@plaplant
Copy link
Member

@JBorrow is the idea that the only thing to change would be a different return code (i.e., 404 vs. 200), but otherwise the rest is similar? I'm open to that, as long as the webserver makes it clear that the issue is specifically that no files matched the query. For me personally I typically associate a 404 error with "unclear user error", which might not be the most useful result in this case.

@JBorrow
Copy link
Member Author

JBorrow commented Jan 26, 2024

Yeah, I guess the question is in this scenario:

  • The user provides a set of query parameters
  • Those parameters are used to search the database
  • There are zero matching results

What should this code be?

  • 200: Success - provide an empty list of search responses.
  • 404: Not found - error out

I've already ran into a development and testing issue here where I thought I was finding zero results but actually I just mis-spelled the name of the endpoint...

@JBorrow
Copy link
Member Author

JBorrow commented Jan 29, 2024

I am going to merge this and then refactor the endpoint to return an empty list when nothing is found. That makes the most sense to me. It's not an error for no file to match a search constraint.

@JBorrow JBorrow merged commit 3c6400c into librarian-v2 Jan 29, 2024
3 checks passed
@JBorrow JBorrow deleted the JBorrow/issue29 branch February 23, 2024 18:51
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.

Implement file searching Refactor CLI and associated tests New Client
2 participants