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

⚡️ feat(middleware): Add functionality to cache homepage search results #317

Merged
merged 1 commit into from
Jun 15, 2024

Conversation

maxachis
Copy link
Contributor

@maxachis maxachis commented Jun 2, 2024

Fixes

Description

  • Added new middleware file homepage_search_cache.py to handle caching of search results for agencies without homepage URLs
  • Added SQL queries to retrieve agencies without homepage URLs and update the search cache
  • Implemented functions to get agencies without homepage URLs and update search cache
  • Added new resource HomepageSearchCache in resources/HomepageSearchCache.py to handle API requests for retrieving and updating search cache

Testing

  • Not sure! I could set up an integration test for it, or we could deploy it and see if it works/doesn't break things when I reconfigure the data source identification code to work with it, or try some unknown third option.

Performance

  • Creation of new endpoint. As untested, performance impact unknown, but likely minimal.

Docs

- Added new middleware file `homepage_search_cache.py` to handle caching of search results for agencies without homepage URLs
- Added SQL queries to retrieve agencies without homepage URLs and update the search cache
- Implemented functions to get agencies without homepage URLs and update search cache
- Added new resource `HomepageSearchCache` in `resources/HomepageSearchCache.py` to handle API requests for retrieving and updating search cache
Copy link
Contributor

@josh-chamberlain josh-chamberlain left a comment

Choose a reason for hiding this comment

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

nice! could you please add a section in the docs for this endpoint? that will also take care of PR review—if a reviewer can use the doc to hit the endpoint successfully, that's good.

@maxachis
Copy link
Contributor Author

maxachis commented Jun 3, 2024

@josh-chamberlain Done! Found here: Police-Data-Accessibility-Project/docs#15

Note that I am very unfamiliar with this format, and did not know how to test it. Additionally, because it was fairly redundant, I used ChatGPT to make the first draft. I anticipate this will need editing in the near future, especially as we test it; I'm just not sure how to do that at the moment.

@josh-chamberlain
Copy link
Contributor

@maxachis

  1. on docs formatting, the reason it looks weird like that is because we're using gitbook and it's their ~proprietary format. I find making these changes is much easier in the WYSIWYG editor, which is something I hate to say so it must be true. You should be in the gitbook org, and this link will take you to the edit view: https://app.gitbook.com/@pdap/s/pdap/

It's a little weird at first, but the formatting will make a lot more sense.

  1. I should have thought of this before—did you consider modifying the agencies endpoint to accept an arg for the blank homepage, rather than creating a new endpoint?

@maxachis
Copy link
Contributor Author

maxachis commented Jun 6, 2024

@maxachis

  1. on docs formatting, the reason it looks weird like that is because we're using gitbook and it's their ~proprietary format. I find making these changes is much easier in the WYSIWYG editor, which is something I hate to say so it must be true. You should be in the gitbook org, and this link will take you to the edit view: https://app.gitbook.com/@pdap/s/pdap/

It's a little weird at first, but the formatting will make a lot more sense.

  1. I should have thought of this before—did you consider modifying the agencies endpoint to accept an arg for the blank homepage, rather than creating a new endpoint?

@josh-chamberlain
For 1: Is that mentioned somewhere in the documentation? Might be useful to add if not.
For 2: The challenge is that this is specifically for recording a multitude of possible homepages, without a clear idea of which is most viable. Currently, it defaults to finding 10 possible homepages per agency. So we can't easily update the agencies row itself.

@josh-chamberlain
Copy link
Contributor

@maxachis for 1. I left a comment on the docs PR
2. oh, right—mixed up the context! thanks

@maxachis
Copy link
Contributor Author

@josh-chamberlain Made a Docs Pull Requests, which I've linked in the main post!

@josh-chamberlain
Copy link
Contributor

@maxachis thank you! I approved, since the button was just looking at me, but if you are making docs in support of a PR I do not mind if you just reference + self-merge.

@maxachis maxachis merged commit 46abf7a into main Jun 15, 2024
10 checks passed
@maxachis maxachis deleted the mc_v1_agency_search_cache branch June 15, 2024 21:27
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.

2 participants