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

Add api for search #432

Merged
merged 2 commits into from
Aug 28, 2023
Merged

Add api for search #432

merged 2 commits into from
Aug 28, 2023

Conversation

gjreda
Copy link
Collaborator

@gjreda gjreda commented Aug 28, 2023

fixes #428

@gjreda gjreda requested a review from cguedes August 28, 2023 16:58
@codecov
Copy link

codecov bot commented Aug 28, 2023

Codecov Report

Merging #432 (fffe68e) into main (7209b2e) will decrease coverage by 0.02%.
The diff coverage is 15.00%.

@@            Coverage Diff             @@
##             main     #432      +/-   ##
==========================================
- Coverage   77.70%   77.69%   -0.02%     
==========================================
  Files         190      190              
  Lines       11564    11570       +6     
  Branches     1115     1115              
==========================================
+ Hits         8986     8989       +3     
- Misses       2562     2565       +3     
  Partials       16       16              
Files Changed Coverage Δ
python/generate_schema.py 0.00% <0.00%> (ø)
python/web.py 0.00% <0.00%> (ø)
python/sidecar/http.py 81.28% <60.00%> (-0.65%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@gjreda gjreda marked this pull request as ready for review August 28, 2023 17:47
Copy link
Collaborator

@cguedes cguedes left a comment

Choose a reason for hiding this comment

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

All good. Just a comment about the name parameter.

api.mount("/api/projects", http.project_api)
api.mount("/api/meta", http.meta_api)
api.mount("/api/settings", http.settings_api)
api.mount("/api/sidecar", http.sidecar_api, name="sidecar")
Copy link
Collaborator

Choose a reason for hiding this comment

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

@gjreda why do we have this name parameter?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Starlette optionally uses it for some stuff under the hood. I thought it might show up in the logs, but alas it doesn't.

Anyway, I just decided to leave it for now as I suspect it might be handy later on.

@gjreda gjreda merged commit 5d3e1c5 into main Aug 28, 2023
10 checks passed
@gjreda gjreda deleted the 428-backend-adds-api-for-search branch August 28, 2023 22:12
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.

Backend adds API for /search
2 participants